kibana
kibana copied to clipboard
elastic
[Discover] Fix `useCustomization` returning `undefined` on first render
Summary
This PR fixes an issue where the useCustomization hook in Discover always returns undefined on the first render, which can have unintended consequences. This went unnoticed before, but the work in #166406 revealed the issue, resulting in the Logs Explorer flyout initializing to the wrong width.
With this fix, the width initializes correctly, but the default value of 60% causes the layout to become crammed, so I updated it to what seemed like a more reasonable default of 650px. I can change this to something else if we feel there's a better default.
Before (60%):
Now (650px):
Checklist
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials
- [x] Unit or functional tests were updated or added to match the most common scenarios
- [ ] Flaky Test Runner was used on any tests changed
- [ ] Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
- [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
- [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
- [ ] This was checked for cross-browser compatibility
For maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately
The fix for the undefined customization LGTM, regarding the default size for the flyout, the screenshot shows an example with a simple document, while having more complex logs can result in a (IMO) too compact view.
Another visual aspect is that a fixed width will result in a too-invasive flyout for small screens, while for wide-screen users there could be better space usage.
I would propose having a responsive width using something like max(650px, 40%); for the width computation.
This would let wide-screen users have more space on the flyout to visualize details while having a cap on the flyout when the screen size is smaller, wdyt?
https://github.com/elastic/kibana/assets/34506779/74a84379-d112-416f-85f9-329558330187
Finally, I wonder what is the default flyout width for Discover. It might be a good moment for the flyout (One Discover 🚀) to have a unique responsive size and remove the customization from the logs explorer.
@tonyghiani Thanks for the feedback!
the screenshot shows an example with a simple document, while having more complex logs can result in a (IMO) too compact view.
Is that screenshot from this PR? I'm asking because it's displaying as an overlay flyout, but Logs Explorer and Discover should now use a push flyout instead. I agree it does seem too compact in that screenshot, but it looks like it's using the default Discover width before the push flyout and this fix were added.
Another visual aspect is that a fixed width will result in a too-invasive flyout for small screens, while for wide-screen users there could be better space usage.
I'd agree in the case of a fixed width, but the new flyout isn't fixed and can be resized by users. It also will convert to a overlay flyout on smaller screen sizes and still allow resizing up to 90vw.
I would propose having a responsive width using something like
max(650px, 40%);for the width computation.
Since the new flyout is resizable, I'm not sure this would work as intended, but we could give it a try.
Finally, I wonder what is the default flyout width for Discover. It might be a good moment for the flyout (One Discover 🚀) to have a unique responsive size and remove the customization from the logs explorer.
Agreed that this would be a good idea, and something we can definitely soon if not now. The default we currently use in Discover is 544px.
:green_heart: Build Succeeded
- Buildkite Build
- Commit: 97eebe06ffa98b5b0f8c487561db421c16caa4b3
- Kibana Serverless Image:
docker.elastic.co/kibana-ci/kibana-serverless:pr-181788-97eebe06ffa9 - Observability Deployment
Metrics [docs]
Public APIs missing comments
Total count of every public API that lacks a comment. Target amount is 0. Run
node scripts/build_api_docs --plugin [yourplugin] --stats commentsfor more detailed information.
| id | before | after | diff |
|---|---|---|---|
discover |
99 | 101 | +2 |
Canvas Sharable Runtime
The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.
| id | before | after | diff |
|---|---|---|---|
module count |
- | 5871 | +5871 |
total size |
- | 6.7MB | +6.7MB |
Page load bundle
Size of the bundles that are downloaded on every page load. Target size is below 100kb
| id | before | after | diff |
|---|---|---|---|
discover |
34.9KB | 34.9KB | +36.0B |
logsExplorer |
56.5KB | 56.5KB | -2.0B |
| total | +34.0B |
History
- :green_heart: Build #206443 succeeded 06b966ad427029b3d0a0e05212cc11dfda8d504c
- :green_heart: Build #206202 succeeded 0beea7ac3c8061009a5ec8b8283c86c20946fd60
- :yellow_heart: Build #206193 was flaky d5ac85b3c9ca2ca93443b3a524a014a74cb0f214
To update your PR or re-run it, just comment with:
@elasticmachine merge upstream
cc @davismcphee
