Skip to content
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

[EuiPageHeaderContent] Improve responsiveness #8044

Merged
merged 10 commits into from
Sep 26, 2024

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Sep 25, 2024

Summary

closes #8034
closes #8039

Fixes involve mostly just re-ordering DOM and playing around flex CSS and container queries.

As always, I recommend following along by commit as some extra cleanup and Storybook/VRT updates happened here.

Before After

QA

General checklist

  • Browser QA
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
      - [ ] Checked in both light and dark modes
  • Docs site QA - N/A
    • Props have proper autodocs (using @default if default values are missing)
      - [ ] Added documentation
      - [ ] Checked Code Sandbox works for any docs examples
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
      - [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist - N/A, no mobile Figma designs

- Improve/update prop docs

- Improve storybook controls to match actual defaults/options

- add example `children`

- (bonus) memoize styles
- just to make sure I don't regress anything in this PR
+ DOM cleanup - remove unnecessary `EuiFlexItem` around `rightSideItems` since we're writing custom flex CSS in any case
- now possible since we've simplified our DOM

- no idea why `alignItems === 'top'` needs to be its own condition, it's not doing anything special. remove it since this appears to be defunct code
@cee-chen cee-chen marked this pull request as ready for review September 25, 2024 20:55
@cee-chen cee-chen requested a review from a team as a code owner September 25, 2024 20:55
- slight changes are due to font/typography IIRC, this should mostly show that nothing meaningfully broke for major use cases
Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

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

🚢 🐈‍⬛ Works as expected and the code looks good to me. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I really like your approach to adding before/after commits for VRT 👏 🧠

Copy link
Member Author

Choose a reason for hiding this comment

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

It's really helpful for me to confirm I'm actually fixing stuff! And also makes before/after screenshots in the PR description a breeze! 😎

@cee-chen cee-chen enabled auto-merge (squash) September 26, 2024 22:28
@cee-chen cee-chen merged commit 780048c into elastic:main Sep 26, 2024
5 checks passed
@cee-chen cee-chen deleted the page-header-responsiveness branch September 26, 2024 22:50
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants