chore: Finalize drawer animation and visual design #3482
Conversation
…ape-design/components into explore-drawer-animation
…lore-drawer-animation
at-susie
requested review from
johannes-weber and
georgylobko
and removed request for
a team and
johannes-weber
|
|
||
| > .global-drawer-wrapper { | ||
| display: grid; | ||
| min-inline-size: var(#{custom-props.$drawerSize}); |
There was a problem hiding this comment.
min-inline-size and inline-size have already been set for the parent element. Why do we need to set them twice?
There was a problem hiding this comment.
The inline-size seems unnecessary, but the min-inline-size is necessary, otherwise it has an unwanted width changing behavior when it's entering like the attached image.
| @@ -140,4 +98,124 @@ $global-drawer-expanded-mode: #{awsui.$motion-duration-refresh-only-slow} #{awsu | |||
| display: flex; | |||
| align-items: center; | |||
| } | |||
|
|
|||
| &.drawer-global { | |||
There was a problem hiding this comment.
Note: When I tried to commit the changes from your patch file, our internal linter pointed out specificity conflicts. So I needed to move the whole block to bottom.
There was a problem hiding this comment.
That should not be a case. These style were there before, eslint should be happy with them
|
|
||
| > .global-drawer-wrapper { | ||
| inline-size: 100%; | ||
| grid-template-columns: 8px 1fr; |
There was a problem hiding this comment.
Can we replace this magic number with a token?
There was a problem hiding this comment.
Thank you for the callout. Updated and pushed the fix
|
|
||
| > .global-drawer-wrapper { | ||
| display: grid; | ||
| min-inline-size: var(#{custom-props.$drawerSize}); |
There was a problem hiding this comment.
The question still stands: do we really need to set the inline size twice? Based on our earlier discussion, we agreed that it’s not necessary
There was a problem hiding this comment.
While inline-size is not necessary, which I removed, the min-inline-size is still necessary otherwise the unwanted width change happens.
Uploading Screen Recording 2025-05-13 at 15.54.49.mov…
There was a problem hiding this comment.
Ok, should we remove min-inline-size from the parent selector then?
There was a problem hiding this comment.
Yup, I removed it in this commit
Description
This PR:
Related links, issue #, if available: n/a
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.