-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Patterns: Ajust the space in the pattern explorer list #45730
Conversation
Open in CodeSandbox Web Editor | VS Code | VS Code Insiders |
Size Change: +10 B (0%) Total Size: 1.29 MB
ℹ️ View Unchanged
|
Would love a designer eye but looks good to me. |
Looks like a good small iteration, the only niggle being is the right edge of patterns not lining up with the right edge of the close button, as it does in trunk: I think it should line up, so I'd appreciate not adding the padding on the right. In general I agree there should be padding all around, and I think there's room for a refactor. None of these are necessary for this PR, but just noting that there's a lot of unloved pieces: the hover style shouldn't be a 2px border, it could be a light blue background. The focus style has a very big border radius, it should be 2px and sit closer to the pattern itself. Lots to do! But thank you for the PR, with a right side padding fix, this is a big improvement! |
Thank you for the review, @jasmussen ! Is there anything additional about this PR that should be addressed? If so, I would like to include the space issues you mentioned 👍 |
The only thing to address is to not add padding on the right side, I think it's better to keep that zero as is. What do you think? |
I have not added padding on the right side in this PR. Perhaps you are referring to this scroll bar? This is displayed in the scrollable area in the Windows OS. This is also true for trunk. In Mac OS, scrollbars do not appear unless scrolled and the bar floats. However, Windows, as you can see, reserves an area the width of the scrollbar. |
Ah, my apologies, then merge, thanks again! |
Fix: #45701
What?
This PR applies the appropriate spaces in the pattern explorer list. This also fixes that the focus outline gets cut at the top reported in #45701
How?
24px
space at the top of the pattern list, the same space as the sidebar. This fixes the outline cutoff when focused.32px
. This creates an appropriate space between the pattern list and the sidebar.24px
to the bottom of the pattern list, the same as the top. This may not be required, but the bottom of the sidebar also has24px
of space.Screenshots
Before
After