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

Fleet UI: Updated styles to license expiration banner #18856

Merged
merged 2 commits into from
May 29, 2024
Merged

Conversation

RachelElysia
Copy link
Member

@RachelElysia RachelElysia commented May 8, 2024

Issue

Cerra #17860

Description

  • Banner now inline of main content pages and not a flash notification
  • This matches the newer style of banners such as the ABM banner

Other

  • Clean up styling of ABM message banner to use <InfoBanner/> component and its styling
  • Remove main-content animation that is making the banner flash on every page change

Screenshots

Screenshot 2024-05-08 at 2 54 11 PM Screenshot 2024-05-08 at 2 53 58 PM Screenshot 2024-05-08 at 2 53 50 PM Screenshot 2024-05-08 at 2 53 37 PM Screenshot 2024-05-08 at 2 53 28 PM

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
  • Manual QA for all new/changed functionality

@RachelElysia RachelElysia requested a review from a team as a code owner May 8, 2024 18:55
@@ -6,5 +6,8 @@
// of the main-content when there is a banner. (e.g. sandbox mode)
// Without it the main content pushes the banner off the page.
overflow: auto;
animation: fade-in 250ms ease-out;

> :not(.main-content--animation-disabled) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Open for other solutions to not make the banner flash on page change

tldr:
main-content has animation: fade-in 250ms ease-out; applied to it, but looks super weird to have that banner flash over and over on each page click since it's not moving (feels not React like at all!), so I removed the animation from the banners only by putting it in a div wrapper called .main-content--animation-disabled

Copy link
Contributor

Choose a reason for hiding this comment

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

see above

@jacobshandling
Copy link
Contributor

👀

/>
)}
/>
<div className={`${baseClass}--animation-disabled`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Would React.memo izing the components prevent those undesired rerenders/animations?

@@ -6,5 +6,8 @@
// of the main-content when there is a banner. (e.g. sandbox mode)
// Without it the main content pushes the banner off the page.
overflow: auto;
animation: fade-in 250ms ease-out;

> :not(.main-content--animation-disabled) {
Copy link
Contributor

Choose a reason for hiding this comment

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

see above

@RachelElysia
Copy link
Member Author

Merging before freeze. TODO: look into memoization to prevent rerenders

@RachelElysia RachelElysia merged commit 5e61843 into main May 29, 2024
9 checks passed
@RachelElysia RachelElysia deleted the license-banner branch May 29, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants