Structure of React components: best practice

Hi, I’ve just finished the Random Quote Machine, which you can find it here:

At the beginning I made a unique React Component with all the elements, but than, actually, I don’t know why exactly (maybe I thought it was a better practice), I divided it into components.
Now I’m styling them, but the issue I encountered, is that for example the flex-box is obviously applied to the <div> container of each element, and not directly to the element inside.
So, my questions are:

  1. Is it better to create a unique component or a divided component, in this case?
  2. And in general? I think, at a certain point, I have to divide it as well. So in these cases, how can I style the component, like it would be a normal html stylesheet? Should I style starting from each <div> container? For example, instead of styling directly the “tweet-quote” element, should I style the
    that wraps it?

Thanks

Nicolò

I’m not completely sure, what you mean.

Looking at your code, it looks pretty good. There is some slight redundancy. For example, here:

const Text = (props) => {
  return (
    <div id="text">
      {props.text}
    </div>
  )
}
const Author = (props) => {
  return (
    <div id="author">
      {props.author}
    </div>
  )
}

I would have just made a text component, The only difference between them is the ids. I would have had:

const Text = (props) => {
  return (
    <div id={props.id}>
      {props.text}
    </div>
  )
}

or more streamlined:

const Text = ({text, id}) => (
  <div id={id}>
    {text}
  </div>
)

then I would invoke them with

          <Text text={this.state.currentQuote} id="text" />
          <Text text={this.state.currentAuthor} id="author" />

I also notice this:

      <div id="quote-box" style={{ backgroundColor: this.state.background }}>
        <div id="card">

I’m not sure why we’re wrapping a div inside a div here. It may be needed, I don’t know, it’s just looks a bit odd, unless there is some important styling reason.

Does that answer your question? If not, please rephrase your question, maybe provide some examples.

1 Like

It’s probably easier to do the former because it’s one contained thing, possibly with the exception of the new quote button and definitely with the exception of the tweet button (which has a specific task and logic).

Too many wrapper div’s, as @kevinSmith suggests. There are specific HTML elements for the thing you are doing as well. So for example, for a card component you can just drop in and pass the state values to as props:

function QuoteCard (props) {
  return (
    <figure id="card">
      <h1 id="title">Quote of the day</h1>
      <blockquote id="text">
        <p>{props.currentQuote}</p>
      </blockquote>
      <figcaption id="author">{props.currentAuthor}</figcaption>
      <button type="button" onClick={props.handleChange}>New quote</button>
      <TweetButton {/* props here */}>Tweet quote</TweetButton>
    </figure>
  )
}

I’d use classes instead as well, not IDs. And I’d make them obvious, something like

.quotecard
.quotecard__title
.quotecard__quote
.quotecard__citation
.quotecard__refresh-button
.quotecard__tweet-button

That’s more a CSS/HTML thing, and depends on how you want to do it. As few (and as simple) styling rules as you can get away with if at all possible. All context sensitive though

1 Like

Actually yes, this is only to change the background when a new quote is generated. I found this way to do it, maybe there is something better.

Thanks, this is for sure a good tip!

Yes, I do it.

Actually if I should style my components in this pen, the rendered structure of my html page it would be like the following (i deleted some data to make it cleaner):

<div id="quote-box">
  <div id="card">
    <h1 id="title">Quote of the day</h1>
      <Text  />
    <Author  />
    <NewQuoteButton  />
    <TwitterButton />
  </div>
</div>

Which actually is:

<div id="quote-box">
  <div id="card">
    <h1 id="title">Quote of the day</h1>
    <div id="text">
        {props.text}
    </div>
    <div id="author">
        {props.author}
    </div>
    <div id="new-quote-cont">
        <button id="new-quote" onClick={props.new}>New quote</button>
    </div>
    <div id="tweet-quote-cont">
        <a id="tweet-quote" onClick={props.new}>New quote</a>
    </div>
</div>

The problem I encountered is actually ont the #new-quote and #tweet-quote, because the display-flex that I apply on the #quote-box, doesn’t reach the two element, which are not direct child.

Yes, they are nested. Do they need to be? Is there some styling that you are doing on the div that can’t be applied to the child? Sometimes the answer is yes, often it is no.

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.