-
Notifications
You must be signed in to change notification settings - Fork 188
feat: Left global panel #3660
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
feat: Left global panel #3660
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3660 +/- ##
==========================================
+ Coverage 97.07% 97.08% +0.01%
==========================================
Files 836 840 +4
Lines 24234 24464 +230
Branches 8506 8633 +127
==========================================
+ Hits 23524 23751 +227
- Misses 662 663 +1
- Partials 48 50 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
12fc856
to
30c4e35
Compare
d647595
to
4f8d98b
Compare
3e3fd1c
to
71fb963
Compare
Co-authored-by: Boris Serdiuk <[email protected]>
a6c97ea
to
b83d12c
Compare
<Transition nodeRef={drawerRef} in={show} appear={show} mountOnEnter={true} timeout={250}> | ||
{drawerTransitionState => { | ||
return ( | ||
<Transition nodeRef={drawerRef} in={isExpanded} timeout={250}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to separately manage expanded and show animations, because we need to keep the resize panel until expanded animation transition is not entered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect we take a P1 action item to refactor this code and get rid of the <Transition>
component to make everything simpler.
I made a POC for that: #3777
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This POC works fine for opening the drawer, but does not implement closing animation.
Anyway, agreed, I'll create an action item for that.
// maybe we need to introduce an additional prop in the component to make it more flexible? | ||
// stylelint-disable-next-line @cloudscape-design/no-implicit-descendant | ||
.ai-drawer-slider-handle { | ||
transform: translate(-4px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will probably be changed as a "cosmetic changes follow-up"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Legend
⛳ – these two comments needed to be addressed before merge
Everything else can be a follow up
<svg width="94" height="24" viewBox="0 0 94 24" fill="none" focusable="false" aria-hidden="true"> | ||
<rect width="94" height="24" rx="4" fill="url(#paint0_linear_145_32649)"/> | ||
<path d="M21.0848 7.31327L15.9128 4.25035C15.3485 3.91655 14.649 3.91655 14.0848 4.25035L8.914 7.31327C8.34976 7.64706 8 8.26965 8 8.93724V15.0631C8 15.7307 8.34976 16.3533 8.914 16.687L14.086 19.75C14.3675 19.9162 14.6844 20 15 20C15.3156 20 15.6325 19.9162 15.914 19.75L21.086 16.687C21.6502 16.3533 22 15.7307 22 15.0631V8.93724C22 8.26965 21.6502 7.64706 21.086 7.31327H21.0848ZM15.3034 18.6673C15.1158 18.7786 14.8818 18.7786 14.6941 18.6673L9.52333 15.6044C9.33565 15.4931 9.21866 15.2856 9.21866 15.0631V8.93724C9.21866 8.71471 9.33565 8.50718 9.52333 8.39591L14.6941 5.33299C14.788 5.27674 14.894 5.24923 14.9988 5.24923C15.1036 5.24923 15.2096 5.27674 15.3034 5.33299L20.4755 8.39591C20.6631 8.50718 20.7801 8.71471 20.7801 8.93724V14.7018L16.3174 12.0589V11.4363C16.3174 11.3026 16.2479 11.1788 16.1346 11.1113L15.1828 10.5475C15.0695 10.4799 14.9305 10.4799 14.8172 10.5475L13.8654 11.1113C13.7521 11.1788 13.6826 11.3026 13.6826 11.4363V12.564C13.6826 12.6978 13.7521 12.8215 13.8654 12.889L14.8172 13.4529C14.9305 13.5204 15.0695 13.5204 15.1828 13.4529L15.7068 13.1428L20.1659 15.7882L15.3047 18.6673H15.3034Z" fill="white"/> | ||
<path d="M34.036 16L33.4 14.02H30.4L29.788 16H28.084L31.06 7.684H32.86L35.824 16H34.036ZM30.772 12.82H33.04L31.9 9.184L30.772 12.82ZM44.3382 16V11.848C44.3382 11.536 44.2702 11.308 44.1342 11.164C44.0062 11.012 43.8022 10.936 43.5222 10.936C43.0502 10.936 42.5782 11.064 42.1062 11.32V16H40.5102V11.848C40.5102 11.536 40.4422 11.308 40.3062 11.164C40.1782 11.012 39.9742 10.936 39.6942 10.936C39.2222 10.936 38.7502 11.064 38.2782 11.32V16H36.6822V9.88H38.0022L38.1462 10.48C38.5622 10.208 38.9422 10.012 39.2862 9.892C39.6302 9.764 39.9862 9.7 40.3542 9.7C41.0822 9.7 41.5902 9.964 41.8782 10.492C42.6622 9.964 43.4222 9.7 44.1582 9.7C44.7262 9.7 45.1622 9.856 45.4662 10.168C45.7782 10.48 45.9342 10.92 45.9342 11.488V16H44.3382ZM51.0638 16L50.9438 15.436C50.6798 15.668 50.3798 15.852 50.0438 15.988C49.7158 16.124 49.3918 16.192 49.0718 16.192C48.4958 16.192 48.0318 16.024 47.6798 15.688C47.3278 15.352 47.1518 14.908 47.1518 14.356C47.1518 13.764 47.3678 13.292 47.7998 12.94C48.2398 12.58 48.8158 12.4 49.5278 12.4C49.9438 12.4 50.3878 12.456 50.8598 12.568V11.92C50.8598 11.536 50.7758 11.272 50.6078 11.128C50.4478 10.976 50.1558 10.9 49.7318 10.9C49.0758 10.9 48.3558 11.028 47.5718 11.284V10.192C47.8678 10.04 48.2358 9.92 48.6758 9.832C49.1158 9.744 49.5598 9.7 50.0078 9.7C50.8158 9.7 51.4078 9.864 51.7838 10.192C52.1598 10.512 52.3478 11.024 52.3478 11.728V16H51.0638ZM49.5278 15.088C49.7358 15.088 49.9558 15.044 50.1878 14.956C50.4278 14.86 50.6518 14.728 50.8598 14.56V13.48C50.4438 13.408 50.0998 13.372 49.8278 13.372C49.0678 13.372 48.6878 13.668 48.6878 14.26C48.6878 14.524 48.7598 14.728 48.9038 14.872C49.0558 15.016 49.2638 15.088 49.5278 15.088ZM53.4919 16V14.848L56.7559 11.08H53.6119V9.88H58.5079V11.032L55.2199 14.8H58.5559V16H53.4919ZM62.2981 16.18C61.3541 16.18 60.6181 15.896 60.0901 15.328C59.5621 14.752 59.2981 13.952 59.2981 12.928C59.2981 11.912 59.5621 11.12 60.0901 10.552C60.6181 9.984 61.3501 9.7 62.2861 9.7C63.2301 9.7 63.9661 9.988 64.4941 10.564C65.0221 11.132 65.2861 11.928 65.2861 12.952C65.2861 13.968 65.0221 14.76 64.4941 15.328C63.9661 15.896 63.2341 16.18 62.2981 16.18ZM62.2981 14.944C63.2101 14.944 63.6661 14.28 63.6661 12.952C63.6661 11.608 63.2061 10.936 62.2861 10.936C61.3741 10.936 60.9181 11.6 60.9181 12.928C60.9181 14.272 61.3781 14.944 62.2981 14.944ZM70.5805 16V11.968C70.5805 11.608 70.5045 11.348 70.3525 11.188C70.2005 11.02 69.9645 10.936 69.6445 10.936C69.1885 10.936 68.7045 11.092 68.1925 11.404V16H66.5965V9.88H67.9165L68.0725 10.54C68.7845 9.98 69.5605 9.7 70.4005 9.7C70.9765 9.7 71.4165 9.856 71.7205 10.168C72.0245 10.472 72.1765 10.912 72.1765 11.488V16H70.5805ZM76.6628 11.836C76.6628 10.932 76.8268 10.152 77.1548 9.496C77.4908 8.84 77.9588 8.34 78.5588 7.996C79.1668 7.652 79.8748 7.48 80.6828 7.48C81.5228 7.48 82.2428 7.66 82.8428 8.02C83.4508 8.38 83.9108 8.888 84.2228 9.544C84.5428 10.192 84.7028 10.956 84.7028 11.836C84.7028 12.556 84.5988 13.2 84.3908 13.768C84.1828 14.336 83.8788 14.812 83.4788 15.196C83.0788 15.572 82.5988 15.844 82.0388 16.012C82.4228 16.204 82.8028 16.348 83.1788 16.444C83.5628 16.548 84.0188 16.62 84.5468 16.66V18.064C83.8508 17.984 83.1548 17.784 82.4588 17.464C81.7708 17.144 81.1188 16.72 80.5028 16.192C79.7028 16.168 79.0148 15.976 78.4388 15.616C77.8628 15.248 77.4228 14.744 77.1188 14.104C76.8148 13.456 76.6628 12.7 76.6628 11.836ZM78.3428 11.836C78.3428 12.812 78.5428 13.56 78.9428 14.08C79.3428 14.592 79.9228 14.848 80.6828 14.848C81.4428 14.848 82.0228 14.592 82.4228 14.08C82.8228 13.568 83.0228 12.82 83.0228 11.836C83.0228 10.86 82.8228 10.116 82.4228 9.604C82.0228 9.092 81.4428 8.836 80.6828 8.836C79.9228 8.836 79.3428 9.092 78.9428 9.604C78.5428 10.116 78.3428 10.86 78.3428 11.836Z" fill="white"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛳ Does this contain the "Amazon Q" title? need to remove this, because it is a brand
Also, you need to eliminate all "Amazon Q" strings from the PR. Find and replace with something like "AI panel"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
@@ -62,7 +64,8 @@ describe('refresh-toolbar', () => { | |||
}) | |||
); | |||
|
|||
test( | |||
// flakiness on github runner side | |||
test.skip( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When are we going to fix these tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After release. Created a follow-up item in the doc qkCSlAib5IKly
<Transition nodeRef={drawerRef} in={show} appear={show} mountOnEnter={true} timeout={250}> | ||
{drawerTransitionState => { | ||
return ( | ||
<Transition nodeRef={drawerRef} in={isExpanded} timeout={250}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect we take a P1 action item to refactor this code and get rid of the <Transition>
component to make everything simpler.
I made a POC for that: #3777
// to override styles in PanelResizeHandle component | ||
// maybe we need to introduce an additional prop in the component to make it more flexible? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also should become an action item
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a follow-up item in the doc qkCSlAib5IKly
grid-area: ai-drawer; | ||
position: sticky; | ||
|
||
@include theming.dark-mode-only { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this code do? It would be useful to have a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored, left a comment: f838874
@include theming.dark-mode-only { | ||
block-size: 100%; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs a comment. How is the color mode affecting sizes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood, but we should push back on such design requirements.
It is not expected to change geometry between light and dark mode. It will lead to unpredictable situations.
package.json
Outdated
@@ -162,12 +162,12 @@ | |||
{ | |||
"path": "lib/components/internal/plugins/index.js", | |||
"brotli": false, | |||
"limit": "15 kB" | |||
"limit": "18 kB" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛳ After runtime API refactoring, this size bump is not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
Description
This PR introduces a new left drawer for
AppLayout
component managed through runtime plugin API.Related links, issue #, if available:
q HUtCAtvl351e/Tela-global-panel-CDS-deliveries
q bzPvANA4oXHI#BLU9BAPvW85
How has this been tested?
u/integ tests
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.