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

Visually differentiate H2 from H3 headings in Metabar Table of Contents #7275

Open
bmuenzenmeyer opened this issue Nov 22, 2024 · 11 comments · May be fixed by #7385
Open

Visually differentiate H2 from H3 headings in Metabar Table of Contents #7275

bmuenzenmeyer opened this issue Nov 22, 2024 · 11 comments · May be fixed by #7385

Comments

@bmuenzenmeyer
Copy link
Collaborator

Visually indicate the difference between an H2 and H3 heading.
This could be indentation or other textual formatting. Whatever it is, make sure it is responsive and does not create a lot of flow problems. For instance, do not indent many many pixels, as that just makes the headings wrap and become longer.


You should use smaller headings or we should limit depth of what goes on Table of Contents

I like the heading hierarchy of the content - it'd be nice if we visually indicated the indentation level. I think H2 and H3 only.

Look at content like https://nodejs.org/en/learn/test-runner/mocking for similar needs.

Originally posted by @bmuenzenmeyer in #7215 (comment)

@AugustinMauroy
Copy link
Member

+1 for that

@nafisreza
Copy link

Hello, I am interested to work on this. Do you think adding text-indent: 10px on the h3 will solve it?

image

@bmuenzenmeyer
Copy link
Collaborator Author

bmuenzenmeyer commented Nov 27, 2024

visually perhaps, but the headings are not rendered as h2, h3 etc in the metabar, it's a definition list entry

see the code at

{heading.length > 0 && (
<>
<dt>{t('components.metabar.tableOfContents')}</dt>
<dd>
<ol>
{heading.map(head => (
<li key={head.value}>
<Link href={`#${head?.data?.id}`}>{head.value}</Link>
</li>
))}
</ol>
</dd>
</>
- which does have the heading context - but i am yet unsure if it knows of the heading size - some plumbing or additional context values may be necessary when tracing it all the way back

see https://nodejs.org/en/learn/test-runner/mocking

image

<div class="MetaBar_wrapper__Th_dl">
  <dl>
    <dt>Table of Contents</dt>
    <dd>
      <ol>
        <li>
          <a href="#when-and-not-to-mock">When and not to mock</a>
        </li>
        <li>
          <a href="#own-code">Own code</a>
        </li>
        <li>
          <a href="#why">Why</a>
        </li>
        <li>
          <a href="#why-not">Why not</a>
        </li>
        <li>
          <a href="#external-code">External code</a>
        </li>
        <li>
          <a href="#why-1">Why</a>
        </li>
        <li>
          <a href="#why-not-1">Why not</a>
        </li>
        <li>
          <a href="#external-system">External system</a>
        </li>
        <li>
          <a href="#what-to-mock">What to mock</a>
        </li>
        <li>
          <a href="#modules--units">Modules + units</a>
        </li>
        <li>
          <a href="#apis">APIs</a>
        </li>
        <li>
          <a href="#time">Time</a>
        </li>
      </ol>
    </dd>
  </dl>
</div>;

@faridomarAf
Copy link
Contributor

faridomarAf commented Nov 27, 2024

Hello @bmuenzenmeyer
can i push the changes which I had made, if it still not solved. and if it looks good, although it affects on the 'Reading Time' and 'Author' 😊
Screenshot 2024-11-27 at 5 22 31 PM

@yaten2302
Copy link

@AugustinMauroy @bmuenzenmeyer may I work on this issue?
Also, @bmuenzenmeyer , as you mentioned, that the headings are not rendered as h2 or h3... so, is it like, we've to change something more rather than just changing the font size, which won't work simply I assume 👀?
I'll try out this on my local first and then will share my approach here 👍

@AugustinMauroy
Copy link
Member

You can work on we don't assign issue

@yaten2302
Copy link

Hey @AugustinMauroy @bmuenzenmeyer , I've created a PR to resolve this issue. I've increased the font size of the headings, previously it was - text-sm, I've increased it to - font-[18px], could you please have a look?

@bmuenzenmeyer
Copy link
Collaborator Author

#7374 and #7286 attempt to do this by simply changing the styles of every heading in the definition list.

we need a more thorough change. we need the mdx heading data (h2 vs h3) to flow through the whole data model all the way to the UI componentry, so we can visually differentiate them

@bmuenzenmeyer bmuenzenmeyer removed the good first issue Issues for newcomers label Jan 2, 2025
@yaten2302
Copy link

@bmuenzenmeyer , could you please guide that what exactly we need here? Like as for h2 and h3 tags, do we need to make the size of the links as h2 and of the headings as h3?
Also, could you please guide something regarding the how to make the changes? I think that simply increasing the font-size is not doing the needful, so like how do we've to change the MDX in order to get this working?

@bmuenzenmeyer bmuenzenmeyer linked a pull request Jan 3, 2025 that will close this issue
5 tasks
@bmuenzenmeyer
Copy link
Collaborator Author

I proposed a fix within #7385 - i didn't realize the heading.depth was available until I tried myself.

@yaten2302
Copy link

yaten2302 commented Jan 3, 2025

@bmuenzenmeyer , ig you've create a fix this issue, could you please ping me once, if I could help in this is(or any other issue) in any way?

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