Skip to content

chore: turn inline logo & shell headers into h3#7060

Merged
emrysal merged 1 commit intocalcom:mainfrom
p6l-richard:main
Feb 13, 2023
Merged

chore: turn inline logo & shell headers into h3#7060
emrysal merged 1 commit intocalcom:mainfrom
p6l-richard:main

Conversation

@p6l-richard
Copy link
Copy Markdown
Contributor

What does this PR do?

image

Environment: Staging(main branch) / Production

Type of change

  • Chore (refactoring code, technical debt, workflow improvements)

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 12, 2023

@p6l-richard is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
ui ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 12, 2023 at 1:55PM (UTC)

@emrysal
Copy link
Copy Markdown
Contributor

emrysal commented Feb 13, 2023

I'd also like an opinion from @JeroenReumkens on this - I agree with this change but also question whether h3 is the right alternative.

@sean-brydon
Copy link
Copy Markdown
Member

I'd also like an opinion from @JeroenReumkens on this - I agree with this change but also question whether h3 is the right alternative.

Putting my input here too -> I agree with this change - No reason at all for logo to be H1. H3 is fine from a a11y standpoint

Copy link
Copy Markdown
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

Approved as clarified by @sean-brydon

@emrysal emrysal merged commit 9d9fa8e into calcom:main Feb 13, 2023
@JeroenReumkens
Copy link
Copy Markdown
Contributor

The h1 in the shell definitely is a good one. And the logo indeed shouldn't be an h1, perhaps not even a heading at all.
Headings work like book chapters, where h1 is the book title (and the book is the page) — and that is not the Cal.com logo, but rather the title of the page. And h2 is then chapter 1.1, 1.2, 2.1, etc (1 level deep), and the h3 is 1.1.1, 1.2.1 2.1.1 etc.

This also shows that you can't skip levels (so no h3 without an h2), and also proves the point that the logo isn't an h3, because it's not suddenly a sub chapter of anything. So it should actually be a paragraph 😁
But since this PR is already merged, I think the biggest issue is solved, and that is making the right thing an h1. That's by far the most important part 🙂

@emrysal
Copy link
Copy Markdown
Contributor

emrysal commented Feb 13, 2023

Yeah that was my idea as well @JeroenReumkens - I wasn't sure if it was "best" to have h3, but the main problem was with the h1 so was good to merge.

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