-
Notifications
You must be signed in to change notification settings - Fork 867
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
[Page Header] Add responsiveness for page header #8171
[Page Header] Add responsiveness for page header #8171
Conversation
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8171 +/- ##
==========================================
+ Coverage 63.89% 64.05% +0.16%
==========================================
Files 3739 3741 +2
Lines 88762 88683 -79
Branches 13814 13817 +3
==========================================
+ Hits 56718 56810 +92
+ Misses 31446 31261 -185
- Partials 598 612 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Just to confirm, the additional whitespace after removing the popover behavior will be addressed separately? |
|
710647d
to
b36cc04
Compare
Sorry, the space I'm referring to is the whitespace above the area selected in your screenshot. |
e61ab07
to
a0ba3a9
Compare
@virajsanghvi Fixed the spacing issue, and updated the attached video, could you re-review? |
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.
@virajsanghvi This comes with the default EuiFlexItem, to fix it, we need to update in OUI |
Can we not address with using smaller padding sizes on the elements or a css override? Anyways, may be easier to identify options once its pushed. |
a0ba3a9
to
c57c1a5
Compare
Signed-off-by: Zhongnan Su <szhongna@amazon.com>
c57c1a5
to
f5fba22
Compare
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.
Change looks good assuming checks pass
@@ -110,3 +110,11 @@ | |||
#globalHeaderBars:has(.primaryApplicationHeader) { | |||
border-bottom: 1px solid $euiColorLightShade; | |||
} | |||
|
|||
/* Control flex items to wrap when the viewport is less than the medium size */ | |||
@media (max-width: 768px) { |
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.
nit: are $euiBreakpoints available here? If so, could use $euiBreakpoints['m']
instead of the hard coded value
Signed-off-by: Zhongnan Su <szhongna@amazon.com> (cherry picked from commit 636f399) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Zhongnan Su <szhongna@amazon.com> (cherry picked from commit 636f399) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 636f399) Signed-off-by: Zhongnan Su <szhongna@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Issues Resolved
Screenshot
iShot_2024-09-17_01.01.08.mp4
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration