Skip to content

Conversation

@danilo-leal
Copy link
Collaborator

@danilo-leal danilo-leal commented Feb 19, 2024

The commits are as isolated as possible for ease of review. This PR is mostly tiny fixes for adding focus-visible styles and spacing adjustments throughout a couple of docs-infra-related components.

To test it, try tabbing through the table of contents items or the page's body to see improved focus-visible styles, for example.

https://deploy-preview-41191--material-ui.netlify.app/material-ui/getting-started/

@danilo-leal danilo-leal added accessibility a11y design This is about UI or UX design, please involve a designer. scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305). labels Feb 19, 2024
@danilo-leal danilo-leal self-assigned this Feb 19, 2024
@mui-bot
Copy link

mui-bot commented Feb 19, 2024

Netlify deploy preview

https://deploy-preview-41191--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 062881a

borderColor: 'divider',
}}
>
<Button
Copy link
Member

Choose a reason for hiding this comment

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

Feels a bit weird to have nothing do show a distinction between the nav and sponsors. And the explaination about what are those logo can only be inferred from the "Become a Diamond sponsor" after them.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I felt like having both the "Become a Diamond Sponsor" and "Diamond Sponsor" buttons was redundant, particularly as they were taking to, essentially, the same place. I experimented with adding a "Sponsors" nav label, sort of similar to the "Contents" on the top of the TOC, but felt like visitors would be able to understand what these logos are even without a label?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Feb 20, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Feb 20, 2024
Copy link
Collaborator

@zanivan zanivan left a comment

Choose a reason for hiding this comment

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

Looks good! Just a small comment:

I know it is probably because of the border left, but it feels kinda off to have no border radius on this, since everything else has.

Screenshot 2024-02-21 at 11 54 32

Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

👌

@danilo-leal
Copy link
Collaborator Author

@zanivan yeah, good call! Definitely a detail I want to treat (and the way I'd probably do something similar to the side nav, with pseudo-elements), but I'll keep this one simple! If I were to throw in a border-radius in there, it'd look janky like this:

Screenshot 2024-02-21 at 13 29 18

@danilo-leal danilo-leal merged commit a78985c into mui:master Feb 21, 2024
@danilo-leal danilo-leal deleted the stray-docs-infra-improvements branch February 21, 2024 16:32
@zanivan
Copy link
Collaborator

zanivan commented Feb 21, 2024

@zanivan yeah, good call! Definitely a detail I want to treat (and the way I'd probably do something similar to the side nav, with pseudo-elements), but I'll keep this one simple! If I were to throw in a border-radius in there, it'd look janky like this:

I thought that might be the case 😅

mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility a11y design This is about UI or UX design, please involve a designer. scope: docs-infra Involves the docs-infra product (https://www.notion.so/mui-org/b9f676062eb94747b6768209f7751305).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants