-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Unified search] Make resizable button as the EUI one #137698
Conversation
Pinging @elastic/kibana-app-services (Team:AppServicesSv) |
@MichaelMarcialis I think I managed to move the styles from eui to our editor. 🤞 |
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.
Amazing, @stratoula! Thanks so much for putting this together. I've left some comments below for your review.
Would it be possible to move the resizable button so that it appears on top of the border that divides the unified search UI and the rest of the page content (see example GIF below)? I think doing so would be a nice touch.
...s/unified_search/public/query_string_input/text_based_languages_editor/resizable_button.scss
Outdated
Show resolved
Hide resolved
...ns/unified_search/public/query_string_input/text_based_languages_editor/resizable_button.tsx
Outdated
Show resolved
Hide resolved
My walking away piece of advice: Don't copy/paste direct EUI styles/code. Ask EUI to export this button at the top level: https://github.com/elastic/eui/blob/main/src/components/resizable_container/resizable_button.tsx It'll keep in sync with any a11y or style changes and you can request general improvements that will benefit all usages. 😸 |
Yes @cchaos you are right. I created an issue here elastic/eui#6100 |
Thanks for that advice, @cchaos. @stratoula, since you've already done the work here, I say going ahead with this PR and then switching when EUI exports the button makes sense to me. The issue you created will remind us not to forget to make the switch. |
@MichaelMarcialis I think I addressed everything! |
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.
Thanks for making these updates, @stratoula. Leaving you a few additional comments below for your review.
Also, I noticed that in expanded mode the space between editor footer and border is smaller than the space space between border and page content. On initial glance, it looks like this might be happening because of the negative margining that exists on the editor footer's EuiFlexGroup
. Would it be possible to fix this issue so that the space between both areas is equal (8px from border)?
...ns/unified_search/public/query_string_input/text_based_languages_editor/resizable_button.tsx
Outdated
Show resolved
Hide resolved
...ns/unified_search/public/query_string_input/text_based_languages_editor/resizable_button.tsx
Outdated
Show resolved
Hide resolved
...ns/unified_search/public/query_string_input/text_based_languages_editor/resizable_button.tsx
Outdated
Show resolved
Hide resolved
...s/unified_search/public/query_string_input/text_based_languages_editor/resizable_button.scss
Outdated
Show resolved
Hide resolved
…-ref HEAD~1..HEAD --fix'
…la/kibana into unified-resizable-button
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.
Hey, @stratoula. I've opened a quick design PR for this branch. Once that gets merged in, this looks good from my perspective. Approving now so I don't hold you up further. Thanks!
Suggested Resizable Button Changes
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
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.
Did not test locally, but code changes LGTM
Summary
Part of #136950
Make the custom resizable button to look and feel like the EUIResizableButton (not exported by EUI).