Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of component in Section leads to readability issues in large code #10

Open
drewpc opened this issue Jun 21, 2021 · 2 comments
Open

Comments

@drewpc
Copy link

drewpc commented Jun 21, 2021

Would you consider a PR that removes the need for a component attribute in the <Section> component? It would favor using all children instead. This architecture change would alter the below example:

    <Section
      component={
        <div>
          <MyIcon />
          <H>My hx</H>
        </div>
      }
    >
      <Section component={<H>My hx+1</H>}>
        <p>...</p>
      </Section>
      <Section component={<H>My hx+1</H>}>
        <ChildComponent />
      </Section>
    </Section>

To be more readable in a large code base, like this::

    <Section>
      <div>
        <MyIcon />
        <H>My hx</H>
      </div>
      <Section>
        <H>My hx+1</H>
        <p>...</p>
      </Section>
      <Section>
        <H>My hx+1</H>
        <ChildComponent />
      </Section>
    </Section>
@alexnault
Copy link
Owner

alexnault commented Jun 22, 2021

Hi @drewpc!

This is how react-headings used to be, in version 2.x. And unfortunately, this solution came with its downsides...

  1. It was very easy to miss a heading, and therefore break the hierarchy:
<Section>
  <Section>
    <H>This is a hx+1</H> {/* problem: we skipped hx */}
  </Section>
</Section>
  1. Headings could also easily be inserted after their sub-headings, again breaking the hierarchy:
<Section>
  <Section>
    <H>My hx+1</H>
  </Section>
  <H>My hx</H> {/* problem: hx comes after hx+1 */}
</Section>

Those are the ones that come to mind right now.

While seemingly trivial, the lack of structure and guarantees created confusion for some, myself included.

Although react-headings 3 is not perfect, it helped mitigate those issues. Do you know how we could make the API more readable while avoiding the problems mentioned?

@drewpc
Copy link
Author

drewpc commented Jun 22, 2021

Oh great! Those are issues I hadn't considered previously. In my tests, the second issue worked fine as is--a header coming after a section rendered with the proper level. The first issue required a bit of rethinking, but I was able to make it work with a state variable that tracks whether or not an H tag was used in a section.

I'll run it through some tests and submit a PR later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants