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

[docs] Place Quick Nav after the main demo in the DOM #845

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vladmoroz
Copy link
Contributor

@vladmoroz vladmoroz commented Nov 20, 2024

Place Quick Nav after the main demo in the DOM so that keyboard focus follows the reading order

https://deploy-preview-845--base-ui.netlify.app/new/components/dialog

@vladmoroz vladmoroz added the docs Improvements or additions to the documentation label Nov 20, 2024
Comment on lines +11 to +13
& + * + * {
clear: left;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A thinker figure looking at the lobotomised owl

@mui-bot
Copy link

mui-bot commented Nov 20, 2024

Netlify deploy preview

https://deploy-preview-845--base-ui.netlify.app/

Generated by 🚫 dangerJS against cc6ae2b

@colmtuite
Copy link
Contributor

@vladmoroz Imo, this won't be expected behaviour when headings become links.

@vladmoroz
Copy link
Contributor Author

@vladmoroz Imo, this won't be expected behaviour when headings become links.

  • Hmm what's the expected behaviour in that case?
  • I wouldn't do heading links personally, I think quick nav is perfectly sufficient for grabbing a link to a section. I like plain, non-clickable headings. Would this be too much of a contentious choice?

@colmtuite
Copy link
Contributor

Discussed on a call:

Imo, headings should be links in some fashion. I appreciate that it's annoying when the actual heading is clickable, it's nice to be able to select or click headings for no reason at all while you're reading. But we could put a link icon beside each heading, or something similar.

Imo, when focus is in the main content pane, it should remain there. Sidebar will/should be considered a separate pane. So focus should either go to sidebar first or last.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants