[Flyout System] Deprecate flyout menu hideTitle prop#9502
[Flyout System] Deprecate flyout menu hideTitle prop#9502angeles-mb merged 4 commits intoelastic:mainfrom
Conversation
mgadewoll
left a comment
There was a problem hiding this comment.
🟢 The code changes are LGTM and the hideTitle behavior works as expected. Thanks for the update!
I only left one tiny nit.
|
@angeles-mb I forgot one thing: Could you please run VRT? There should be updates needed. |
|
@mgadewoll I'm confused, aren't VRT tests run on every PR? Or do they soft fail?
I don't expect changes since for the manager stories loki is being skipped and for the single flyouts the title was already being hidden by default. I'll run them in any case and see what happens. |
Urgh, that part in the wiki is not true. No, VRT is currently not automatically run. 🙈
I ran it briefly on your branch and saw that there were changes. |
I've updated it.
I understand now, those changes you're seeing are related to my previous PR [Flyout System] Add flyout menu display modes which did alter the stories in a more meaningful way (I added custom actions which make the flyout menu to render). I had never run these tests before so these changes are expected then. I've updated the screenshots. |
💚 Build SucceededHistory
cc @angeles-mb |
💚 Build Succeeded
History
cc @angeles-mb |
Summary
This PR:
hideTitleprop inEuiFlyoutMenuProps. The menu title is now hidden by default for all flyouts.EuiFlyoutHeaderwill be the recommended way for visible titles instead. Prop is still honored.flyoutMenuHideTitlecomputation insideuseFlyoutMenuhook since this logic is no longer needed. We hide by default and only show title is prop is set to false, we can rely on the raw prop now.Why are we making this change?
To enforce consistency between all flyouts and between main and child flyouts (previously, main flyouts hid titles by default and child showed them by default).
Closes https://github.com/elastic/kibana-team/issues/3017
Screenshots #
No changes for rows 1, 2, 5 (only warning)
Testing checklist:
Storybook
EuiFlyout > EuiFlyoutMenu > Playground:hideTitletrue)hideTitleto false, menu now renders with a title and header title is not renderedStorybook
EuiFlyout > Flyout Manager > Playground:showCustomActionsresults in both flyouts showing no menu (no visible menu title nor actions, menu hides)Kibana flyout examples:
If you want to test in kibana follow these instructions to test kibana against this branch. Run kibana with
yarn start --no-cache --run-examplesto be able to test Flyout System Examples (reachable via global search bar)hideTitlefalse)Impact to users
Users setting the prop will notice no behavior change. Users not using the prop will notice that child flyouts now never render a title inside the flyout menu.
QA
Remove or strikethrough items that do not apply to your PR.
General checklist
Checked in both light and dark modesChecked in both MacOS and Windows high contrast modesChecked in mobileChecked for accessibility including keyboard-only and screenreader modes@defaultif default values are missing) and playground togglesChecked Code Sandbox works for any docs examplesUpdated visual regression testsIf applicable, added the breaking change issue label (and filled out the breaking change checklist)If the changes unblock an issue in a different repo, smoke tested carefully (see Testing EUI features in Kibana ahead of time)If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)