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

[Lens][Visualizations] library annotation groups listing page #157988

Merged

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented May 17, 2023

Summary

Resolve #141054
Resolve #145744

Screen.Recording.2023-06-05.at.9.42.24.AM.mov
  • Telemetry
  • test page-size changes on listing page
  • empty state on listing page
  • Test permissions
  • flaky test runner
  • permissions on listing page
  • link on saved objects page
  • link to specific annotations?
  • Title click on annotation library
  • remove lens and lns mentions from annotation plugin and visualization-ui-components?
  • hide individual annotation editing
  • dimension button focus rings

Checklist

Delete any items that are not applicable to this PR.

…b.com:drewdaemon/kibana into 141054/library-annotation-groups-listing-page
@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@drewdaemon
Copy link
Contributor Author

@MichaelMarcialis thanks for your design feedback. We're going to have to wait on some of the improvements you've pointed out:

The wireframes propose the use of a table utility bar pattern for displaying object counts and bulk selection actions. Assuming it's not something that clashes with the Shared UX team's content management efforts, are we considering adding this pattern for a future phase? Asking because I was accounting for the inclusion of merge, duplicate, and delete bulk actions in my wireframes.

Rather than a single action to edit, do we want to add actions to duplicate and delete individual annotation groups, as shown in the wireframes? At the moment, there is no way to duplicate annotation groups and deletion requires users to interact with the selection checkboxes first (which isn't ideal).

As @sebelga pointed out, this is dependent on further improvements to the shared-ux components. We met as a team and feel okay about releasing without the merge and duplicate options for 8.9.

The columns for "Data view" and "Usages" (along with its flyout listing of visualizations) proposed in the wireframes are missing from the annotation groups table. Would it be possible to add, or is this being considered for a future phase?

I have added the Data view column, but Usages will have to wait. It will require some technical thinking and, as with the "used-in X number of visualizations" callout on the save modal, I'm not certain of who will take ownership here.

When transitioning from the "Visualizations" tab to the "Annotation groups" tab, a table briefly flashes and then disappears on the page. Would it be possible to prevent that? I think @stratoula mentioned it as well, but adding it here for visibility.

This is a pre-existing behavior of the shared-ux component. I've created an issue for that team: #159507

@andreadelrio andreadelrio self-requested a review June 12, 2023 21:32
Copy link
Contributor

@andreadelrio andreadelrio left a comment

Choose a reason for hiding this comment

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

Design changes LGTM. Noting that some of the improvements that @MichaelMarcialis suggested will be implemented later on.

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Changes LGTM! I tested it locally and works fine!

@drewdaemon
Copy link
Contributor Author

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

I have some comments about little things but I am ok with fixing it during fix-it week. Some of my questions might be not so much things to fix but just me being unaware of decisions we made. Or even not coming from this PR 😅

  1. Dark mode save modal stylings - that's coming from the save modal right and doesn't belong here, right?
Screenshot 2023-06-13 at 10 13 04
  1. There is a memory leak when going from annotations group listing tab to visualization one. Not sure why it only happens this way, but has something to do with urlState.
  2. When on annotation groups listing tab, when navigating away from visualize app, we come back to annotation listing tab. It's probably expected behavior, but I would probably expect coming back to visualizations in this case.
  3. I think this is not related to this PR. Open the same lens visualization with referenced annotation group with two annotations twice, in two tabs. Modify annotation in the first one and save. Switch tab and modify different annotation from the group. Save. Refresh the window. The annotation group gets the initial state and cannot be saved, no matter how many times I try.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #18 / discover/group3 discover sidebar field filtering should show filters by type in text-based view
  • [job] [logs] FTR Configs #36 / graph app feature controls spaces space with no features disabled allows creating a new graph

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboard 376 378 +2
eventAnnotation 22 208 +186
filesManagement 133 135 +2
graph 278 280 +2
lens 898 885 -13
maps 983 985 +2
visualizations 335 338 +3
visualizationUiComponents 78 89 +11
total +195

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
@kbn/content-management-tabbed-table-list-view - 14 +14
@kbn/content-management-table-list 14 - -14
@kbn/content-management-table-list-view - 9 +9
@kbn/content-management-table-list-view-table - 41 +41
eventAnnotation 204 227 +23
visualizationUiComponents 119 159 +40
total +113

Async chunks

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

id before after diff
dashboard 357.6KB 358.6KB +958.0B
eventAnnotation 5.9KB 110.2KB +104.3KB
filesManagement 87.7KB 88.7KB +960.0B
graph 456.4KB 457.3KB +958.0B
lens 1.3MB 1.2MB -23.8KB
maps 2.7MB 2.7MB +969.0B
visualizations 260.1KB 261.1KB +1022.0B
total +85.2KB

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
@kbn/content-management-table-list 5 - -5
@kbn/content-management-table-list-view-table - 4 +4
eventAnnotation 3 4 +1
visualizations 18 19 +1
total +1

Page load bundle

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

id before after diff
eventAnnotation 20.1KB 32.7KB +12.6KB
lens 35.2KB 35.3KB +75.0B
visualizations 56.7KB 56.8KB +95.0B
visualizationUiComponents 31.1KB 38.2KB +7.1KB
total +19.9KB
Unknown metric groups

API count

id before after diff
@kbn/content-management-tabbed-table-list-view - 14 +14
@kbn/content-management-table-list 21 - -21
@kbn/content-management-table-list-view - 9 +9
@kbn/content-management-table-list-view-table - 58 +58
eventAnnotation 204 227 +23
visualizationUiComponents 122 163 +41
total +124

async chunk count

id before after diff
eventAnnotation 1 4 +3

ESLint disabled line counts

id before after diff
@kbn/content-management-table-list 2 - -2
@kbn/content-management-table-list-view - 1 +1
@kbn/content-management-table-list-view-table - 1 +1
enterpriseSearch 19 21 +2
eventAnnotation 0 1 +1
securitySolution 410 414 +4
total +7

miscellaneous assets size

id before after diff
eventAnnotation 0.0B 161.8KB +161.8KB

References to deprecated APIs

id before after diff
@kbn/content-management-table-list-view - 2 +2
eventAnnotation 21 47 +26
filesManagement 0 1 +1
total +29

Total ESLint disabled count

id before after diff
@kbn/content-management-table-list 2 - -2
@kbn/content-management-table-list-view - 1 +1
@kbn/content-management-table-list-view-table - 1 +1
enterpriseSearch 20 22 +2
eventAnnotation 1 2 +1
securitySolution 491 495 +4
total +7

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:build-webpack-bundle-analyzer release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TableListView] Expose table only component [Lens] [Annotations] Save annotations to library