Skip to content

docs: add category heading in docs pages #8918

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

Merged
merged 4 commits into from
Jul 10, 2023

Conversation

bleucitron
Copy link
Contributor

I find it useful to have the category displayed at the top of a doc page. It allows the reader to know instantly which category he/she's in.

Screenshot 2023-07-05 at 18 29 06

@benmccann
Copy link
Member

Thanks for the idea. Personally, after browsing around the deployed preview version of the site, I don't think I'm in favor of this change. I find it a bit odd to have two headings and to have a larger heading below a smaller heading.

@PuruVJ
Copy link
Collaborator

PuruVJ commented Jul 5, 2023

Im sort of in favor though. If you tweak the styles, it looks better
CleanShot 2023-07-06 at 01 45 00@2x
CleanShot 2023-07-06 at 01 44 56@2x

@bleucitron
Copy link
Contributor Author

Your styling suggestion looks indeed much better than mine @PuruVJ

Comment on lines 19 to 21
{#if category}
<p class="category" aria-label="category">{category}</p>
{/if}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up doing this, we should remove the aria-label - it doesn't have any effect on <p> elements

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah... i'm not good at a11y, i was trying to give some semantics, but did not really know what tag to choose.

Do you think the <p> alone is good enough, or is there another tag more relevant ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, i just realised there is the aria-description that could fit.

Copy link
Collaborator

@PuruVJ PuruVJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of this! If others also agree, then lets go ahead and merge it cc @dummdidumm @benmccann

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does look better to me

@benmccann benmccann merged commit 258ccf2 into sveltejs:master Jul 10, 2023
@bleucitron bleucitron deleted the docs/category-heading branch August 15, 2023 11:52
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

Successfully merging this pull request may close these issues.

4 participants