snippethtmlMinor
Class to create HTML heading tags in React
Viewed 0 times
headingcreatereacttagsclasshtml
Problem
I have a class which is part of my page layout. It is creating header tag elements (
h1 - h5). Could someone tell me how I could improve it (I know it should be improved, but don't know how)? I have problem with rewriting almost the same code in switch cases. Would rather to create some kind of `` code, but couldn't.import React, {Component} from 'react';
import PropTypes from 'prop-types';
export class TypographyHeader extends Component {
createHeader() {
let txt = this.props.text;
let spaceIndex = txt.indexOf(' ');
let p2 = txt.substr(spaceIndex);
txt =
{txt.substr(0, spaceIndex)}
;
switch(this.props.headerType) {
case 1:
return {txt} {p2};
case 2:
return {txt} {p2};
case 3:
return {txt} {p2};
case 4:
return {txt} {p2};
case 5:
return {txt} {p2};
}
return {txt} {p2};
}
render() {
return (
{this.createHeader()}
);
}
}
TypographyHeader.propTypes = {
headerType: PropTypes.oneOf([1, 2, 3, 4, 5]).isRequired,
text: PropTypes.string.isRequired,
};Solution
Choosing the heading level at runtime:
According to the official docs, you cannot use expressions within JSX types. However, you can use capitalized variables:
Further suggestions:
-
Rename txt
Proposed refactoring:
According to the official docs, you cannot use expressions within JSX types. However, you can use capitalized variables:
const Heading = "h" + 1;
return ...;Further suggestions:
- Rename
TypographyHeadertoTypographyHeadingto avoid confusion with `.
- Rename headerType
tolevel.
-
Rename txt
to text in let txt = this.props.text for consistency's sake.
-
Don't recycle the txt variable by rebinding it to a JSX .
- Consider using
const variable bindings.
-
As there are 6 official heading levels, you might want to relax your propTypes constraints:
headerType: PropTypes.oneOf([1, 2, 3, 4, 5, 6]).isRequired
-
Use destructuring arguments to avoid writing let txt = props.text.
- Make the white space following the
element obvious by appending {" "}`.- Handle heading texts without spaces by splitting according to this suggestion.
Proposed refactoring:
function TypographyHeading({text, level}) {
// Split text into leading and remaining words:
const space = text.indexOf(" ");
const leading = text.substr(0, space);
const remaining = text.substr(space + 1);
// Select heading - :
const Heading = "h" + level;
return (
{leading}
{" "}{remaining}
);
}Code Snippets
const Heading = "h" + 1;
return <Heading>...</Heading>;headerType: PropTypes.oneOf([1, 2, 3, 4, 5, 6]).isRequiredfunction TypographyHeading({text, level}) {
// Split text into leading and remaining words:
const space = text.indexOf(" ");
const leading = text.substr(0, space);
const remaining = text.substr(space + 1);
// Select heading <h1> - <h5>:
const Heading = "h" + level;
return (
<div>
<Heading className="typography_header">
<span className="typography_header__distinctive_el">
{leading}
</span>
{" "}{remaining}
</Heading>
</div>
);
}Context
StackExchange Code Review Q#162693, answer score: 4
Revisions (0)
No revisions yet.