[docs] Improve nav semantics #27138
[docs] Improve nav semantics #27138
Conversation
eps1lon
added
docs
| [theme.breakpoints.up('sm')]: { | ||
| width: 'calc(100% - 175px)', | ||
| }, | ||
| [theme.breakpoints.up('lg')]: { | ||
| width: 'calc(100% - 175px - 240px)', | ||
| }, | ||
| width: 'calc(100% - 175px)', |
There was a problem hiding this comment.
I consider this a win as well. The component is now coupled less to the app drawer (width).
Is this a bug report? Not sure what I'm supposed to do with that statement.
Since this is yet another instance where you are unable to provide a bug report, I'll have to guess that 711ac2e fixed it. |
@eps1lon It's not a bug report. I wanted to highlight the possible implication of the "Skip to content" feature. It's not so obvious to trigger. It seems that you considered it, so all good.
Sorry, I didn't realize it wasn't clear. My assumption was that you forgot to check the layout on mobile, so I raised it.
It seems that the new fix capture what I was referring to as a regression (the toc leaving a placeholder on mobile). While this is fixed, we have a different issue to fix: there should be no horizontal scroll bars no matter the screen width. We have a horizontal scrollbar on mobile and tablet. |
|
I need to work on the composition. The previous composition does not capture the layout correctly. |
I really don't know how else to tell you this. You seem to be under the impression that this is not a You issue but this is absolutely an issue you need to work on. Please write a detailed bug report. I know what horizontal scrollbars are. I know what a tablet is. I think I know what you mean by "mobile". Neither of these things help me understand what the problem is. |
You did not highlight any possible implications. You just vaguely referred to "potential" issues which doesn't say anything. That is not a new insight because any change as potential issues. Without a concrete problem description, you're not saying anything.
What is not obvious to trigger? Displaying the link and activating it? Activating a link is obvious. I'd be in the wrong field if I didn't know how to activate a link. Displaying it is something I do know since that is a field that I specialize in. You can continue to deny that if that makes you feel better. |
There was a problem hiding this comment.
The layout seems correct
Please write a detailed bug report.
The issue was that we had a horizontal scrollbar on mobile, on any of the pages. What other information did you wish to have? It seems that you have fixed it :).
On a side topic, I wouldn't completely discount the value of feedback with partial information. It's similar to what our users are doing on the GitHub issues. It forces us to dive deeper into the issues. I don't know about you, but for me, when it's easy, I don't feel like I learn. Anyway, I'm not saying that partial feedback is great, the time spend investigating an issue might feel like it would better be spent fixing the issue, but I don't think that it's completely worthless.
What is not obvious to trigger?
I meant that the skip nav was implemented by Matt a long time ago. If we ask other members of the team, I doubt they know it exists.
|
I have just noticed this regression: https://deploy-preview-27138--material-ui.netlify.app/guides/migration-v4/
|
Before:
After:
Preview: https://deploy-preview-27138--material-ui.netlify.app/components/box/
Since the ToC
<nav />is related to the<main />element it should be contained within it. Previously both navs were siblings of the<main />Also adjusted the name of the app drawer. The "navigation" term was redundant (it is implied by the landmark type). "main" might've been misleading since it's not the navigation for the "main" element but rather the whole "documentation".