Skip to content

[Guided Onboarding] Make onboarding panel auto height#147333

Merged
cindychangy merged 4 commits intoelastic:mainfrom
cindychangy:guided_onboarding_panel_height
Dec 13, 2022
Merged

[Guided Onboarding] Make onboarding panel auto height#147333
cindychangy merged 4 commits intoelastic:mainfrom
cindychangy:guided_onboarding_panel_height

Conversation

@cindychangy
Copy link
Contributor

@cindychangy cindychangy commented Dec 12, 2022

Fixes issue #144334

Making the panel not take the full height of the screen as is the default of EuiFlyout which we are using.
image

Notes:

  • Realized when reading the docs, that there is a maxWidth prop for the EuiFlyout so I swapped that with the custom style we had
  • Revised the custom styles in panel_styles.ts to achieve this
  • I put a max-height on the panel so there is always room for 1 toast

Let me know if you think there is a better way to approach this.

@cindychangy cindychangy added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Journey/Onboarding Platform Journey Onboarding team v8.7.0 labels Dec 12, 2022
@cindychangy cindychangy requested a review from a team as a code owner December 12, 2022 09:27
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-onboarding (Team:Journey/Onboarding)

@cindychangy cindychangy changed the title [Guided Onboarding] Make onboarding panel auto height #144334 [Guided Onboarding] Make onboarding panel auto height Dec 12, 2022
flyoutContainer: css`
top: 55px !important;
bottom: 25px !important;
bottom: unset !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: would it be possible to delete the property completely?

Copy link
Contributor

Choose a reason for hiding this comment

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

(If it's not possible to delete, it might be worth adding a comment to explain why.)

Copy link
Contributor Author

@cindychangy cindychangy Dec 12, 2022

Choose a reason for hiding this comment

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

@yuliacech @alisonelizabeth - good point, I added a comment to clarify (does not work if we remove these values) - I had one quick question, is this placement the correct place to put it, or should I add it all the way at the top with the commented description:

/**
 *
 * Style overrides for the setup guide dropdown panel.
 * There is currently no existing EUI component that fully supports what we need...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is OK 👍. Not sure if there are any specific guidelines around this, but in this case, I think it's easier to follow next to the CSS.

border-radius: 6px;
inline-size: 480px !important;
height: auto;
height: unset !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, maybe we could delete height?

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @cindychangy! LGTM, left a couple of nits in the comments.

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Verified locally. LGTM. Thanks @cindychangy!

@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
guidedOnboarding 20.8KB 20.8KB +29.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 60 66 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 69 75 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@cindychangy cindychangy merged commit 274ed54 into elastic:main Dec 13, 2022
@cindychangy cindychangy deleted the guided_onboarding_panel_height branch December 13, 2022 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Journey/Onboarding Platform Journey Onboarding team v8.7.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Guided onboarding] Remove fixed height on dropdown panel

5 participants