HiveBrain v1.2.0
Get Started
← Back to all entries
snippethtmlMinor

Class to create HTML heading tags in React

Submitted by: @import:stackexchange-codereview··
0
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:

const Heading = "h" + 1;
return ...;


Further suggestions:

  • Rename TypographyHeader to TypographyHeading to avoid confusion with `.



  • Rename headerType to level.



-
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]).isRequired
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 <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.