Skip to content
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

[TableListView] Refactor to function component #135807

Merged

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Jul 6, 2022

This PR is part of the issue to refactor the <TableListView /> component. (#135020). I decided to split the work in smaller PRs to make it easier to review.

In this initial PR the changes are: converting the previous Class component to a Function component. I've decided to have a reducer to manage the state as it is the best way to make multiple changes to the component state at once and we expect the state management of the component to grow in complexity with future work on the UX redesign.

How to test

Navigate to the dashboard list view and make sure that everything works as expected

  • List the dashboard
  • Query (search bar)
  • Delete one or multiple dashboard
  • Tagging column appear if there are tags
  • Edit a dashboard

Note: This PR is going to the feature branch. Tests will be added later (#135811). This PR is just converting the actual component without adding any new functionalities.

Note 2: There is an issue in EUI with the sorting controlled by the consumer. I've opened an issue (elastic/eui#6032). Until it is fixed, the "updatedAt" column can't toggle between "asc" and "desc".

@sebelga sebelga force-pushed the table-list-view/refactor branch from ac08089 to d1a7682 Compare July 7, 2022 10:41
@sebelga sebelga marked this pull request as ready for review July 7, 2022 10:45
@sebelga sebelga requested review from a team and majagrubic July 7, 2022 10:45
@sebelga sebelga added Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Content Management User generated content (saved objects) management labels Jul 7, 2022
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #31 / ObservabilityApp Observability Rule Details page Page components maps correctly the rule type with the human readable rule type

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
kibanaReact 318 323 +5

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 comments for more detailed information.

id before after diff
kibanaReact 214 187 -27

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
@kbn/test-jest-helpers 8 5 -3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
kibanaReact 209.6KB 209.6KB +4.0B

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 4966 4969 +3

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
kibanaReact 5 4 -1

Page load bundle

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

id before after diff
kibanaReact 62.5KB 62.3KB -135.0B
Unknown metric groups

API count

id before after diff
kibanaReact 251 224 -27

History

  • 💔 Build #55665 failed ac08089b1cbc6d568db040108bd52ca8bb070a7f
  • 💔 Build #55572 failed 283f8bd4a61dd30ead2adbd2b5ec10662c4faa2a

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

Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

Code review only, haven't tested.

@sebelga
Copy link
Contributor Author

sebelga commented Jul 7, 2022

Thanks for the review @majagrubic 👍

@sebelga sebelga merged commit c6c8cd1 into elastic:table-list-view/enhance-ux Jul 7, 2022
@sebelga sebelga deleted the table-list-view/refactor branch July 7, 2022 16:47
@sebelga sebelga self-assigned this Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Content Management User generated content (saved objects) management Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants