-
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
[Table list view] Improve UX (phase 1) #135892
Conversation
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
ddbd8a4
to
91fd89d
Compare
@elasticmachine merge upstream |
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.
DataDiscovery.team code LGTM, tested with 🍜 deployment, just changes in Graph, Table list providing the selection of available Graph saved objects, works as expected 👍
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.
Presentation team changes LGTM!
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.
kibana-gis changes LGTM
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.
Changes to Data Streams test LGTM.
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.
I tested this in Storybook, in Chrome on Mac OSX. I didn't run Kibana nor tested application-specific changes. Shared-ux
changes and conversion to package look good to me. My only question is if we're planning on adding more unit tests to components, like ListLimitWarning
and ConfirmDeleteModal
components.
packages/content-management/table_list/src/components/confirm_delete_modal.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/application/listing/dashboard_listing.test.tsx
Show resolved
Hide resolved
packages/content-management/table_list/src/components/listing_limit_warning.tsx
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
@elasticmachine merge upstream |
Thanks for the review @majagrubic !
Probably not. I prefer component integration tests over unit tests. They give higher confidence in the test coverage.
|
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
miscellaneous assets size
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @sebelga |
This PR is the phase 1 for the work to enhance the UX of the
<TableListView />
. It consist mainly in converting the component exposed by thekibana_react
plugin into a package, refactor and cleanup the interface and integrate the package intoIt contains the following intermediate PR that have already been reviewed
How to test