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] Replace basic table with EuiDataGrid #72504

Closed
wylieconlon opened this issue Jul 20, 2020 · 9 comments · Fixed by #88069
Closed

[Lens] Replace basic table with EuiDataGrid #72504

wylieconlon opened this issue Jul 20, 2020 · 9 comments · Fixed by #88069
Assignees
Labels
Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture

Comments

@wylieconlon
Copy link
Contributor

wylieconlon commented Jul 20, 2020

Edit: Discussion concluded and this is now a recommended improvement that is not really a new feature, but tech debt we should tackle

The current Lens table implementation is very simple and supports no interactivity or custom styling. There are three elements to the discussion of replacing the Lens table with a more powerful table:

  • Are there product requirements that aren't supported by the basic table, but are supported by the data grid?
  • Technical requirements for the table
  • Consistency across Kibana

Unless there is a strong reason to use the data grid in one of these categories, Lens should not switch.

Product requirements that might need to be supported

Based on the table features for Lens:

Feature Basic table Data grid
Changing sort order from table Lens is not using this feature, but could Yes
Align numbers to the right Lens is not using this feature, but could Yes
Reordering columns via drag & drop - Yes, but in a separate menu, WIP PR for in-header
Changing column width - Yes
Subtotal rows Yes, can add a footer row WIP PR
Hierarchical display - -
Grouped rows - -
Full pivot table support - -
Colored backgrounds Yes Yes
Infinite scroll Yes Not yet
Cell-level widgets Yes Yes

The main difference is therefore support for changing column width, reordering columns, and subtotal rows.

Technical requirements

The EuiDataGrid must be performant and stable before we switch.

Consistency across Kibana

Currently, the EuiBasicTable is used in every table visualization that can be embedded in a dashboard, except for the upcoming Discover implementation. If we make a change only in Lens, will this introduce inconsistency?


Conclusion

Based on this analysis of the requirements for Lens tables, I do not think that the benefits outweigh the costs of switching.

cc @elastic/eui-design

@wylieconlon wylieconlon added discuss Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Jul 20, 2020
@cchaos
Copy link
Contributor

cchaos commented Jul 20, 2020

Currently, the EuiBasicTable is used in every table visualization that can be embedded in a dashboard, except for the upcoming Discover implementation. If we make a change only in Lens, will this introduce inconsistency?

The guidelines for EuiBasicTable vs EuiDataGrid usages state that the EuiBasicTable is for simple display of a "list of things in tabular format" while EuiDataGrid is specifically for large amounts of known and unkown data. For example, the correct usage of the basic table is on listing pages like Dashboard and Visualize items. The only reason the dataVis table uses the basic table is because the data grid didn't exist. But all implementations of user-generated table visualizations should use the EuiDataGrid component to be consistent.

Based on this analysis of the requirements for Lens tables, I do not think that the benefits outweigh the costs of switching.

What are the costs?

Looking at the table, to me, it is a huge win for the user if we switch over to EuiDataGrid

  • Only 4 out of the 11 items in the list are supported by the current implementation
  • While 8 out of the 11 items in the list will be available and/or built-in with no customization needed by EuiDataGrid
  • The remainder 3 items aren't supported by either at the moment

I understand that there are performance issues right now, but they are actively being worked on. I'm not advocating for the switch to happen today, but we should consider the fact that we will lose out on a lot of features if we don't even consider switching.


cc @AlonaNadler : Also if you can find our requirements/necessary features for table visualizations to link back to, that would be really helpful. Even future features.

@snide
Copy link
Contributor

snide commented Jul 20, 2020

Infinite scroll

This isn't in basic table. If it exists in Kibana, it's an abstraction.

I would also echo @cchaos' reasoning for using grid. Using a side by side comparison like this is a little tough when we're using boolean classification, since a lot of these read closer to "yes for both, but grid handles this better in this situation". The EUI docs summarize the differences pretty simply:

EuiDataGrid is for displaying large amounts of tabular data. It is a better choice over EUI tables when there are many columns, the data in those columns is fairly uniform, and when schemas and sorting are important for comparison.

End of day this is what data grid was built for and what discover will will be (we're aiming for consistency). Virtualization is being prioritized for data-grid during this current minor. I'm confident performance shouldn't be a blocker, and parallel work can be done by the two teams to deliver for the near future.

This comparison also leaves out some pretty considerable design advantages of grid for the Lens usage, primary that it operates in small spaces extremely well and was built for dashboard usage. The tables only work well in wide formats.

@wylieconlon
Copy link
Contributor Author

I'm trying to use side-by-side comparisons because it doesn't seem like this comparison had been made in the context of Lens or other visualization builders. Since you're suggesting extra criteria that I hadn't considered, could you try formatting it in the context of a comparison? For example:

Feature Basic table Data grid
Header stays at the top No Yes

I derived this example from the screenshot you sent @snide, compared against how Visualize and Lens behave with the basic table:

Screenshot 2020-07-20 16 16 32

This screenshot shows that Visualize is actually hiding the pagination controls, while Lens doesn't use pagination at all and shows the full results in a single page which scrolls. Horizontal scrolling is done by both.

@cchaos
Copy link
Contributor

cchaos commented Jul 20, 2020

Visualize and Lens are not using the same table renderer so we can't bucket them into the same column for comparison against EuiDataGrid.

Visualize is using the legacy agg_table which is still Angular and jQuery while Lens uses EuiBasicTable. This will have to be converted at some point to one of the EUI tables. The simplest way to do this is by using EuiDataGrid to get all the column sort stuff OTB.

The agg table (visualize) also can in fact show pagination, which it does by default. Users have to turn pagination off manually.

Screen Shot 2020-07-20 at 16 36 57 PM

@snide
Copy link
Contributor

snide commented Jul 20, 2020

In that format this is how I'd write it up. There are also some soft comparisons I think need to be considered here. For one, EuiTables are considered mostly feature complete and we are unlikely to add more to it, since they are on purpose built to be simple displays for CRUD interfaces. DataGrid is meant more to be an Excel-like experience, so most of these wants are more compatible with that interface and are likely to be added over time.

Feature EuiBasicTable EuiDataGrid
Designed for unknown user-generated content No Yes
Changing sort order from table Single column sort Multi-column sort
Reordering columns via drag & drop No, not planned Yes, More involved header support otw
User controlled column width No, not planned Yes
Subtotal rows Yes Available in 7.10
Hierarchical display No, not planned No
Grouped rows No, not planned No
Full pivot table support No, not planned No
Custom cell renders (background colors, react nodes for widgets..etc) Yes Yes, but better considered
Virtualization No, not planned Available in 7.10
Works well in small containers No, not planned Yes
Full screen mode No, not planned Yes
Gracefully handles long strings Sometimes, requires a lot of custom css and requires knowledge of the content Always
Allows user to choose styling (borders, padding..etc) No, not planned Yes
Schema level styling (ex: right-align numbers) and expansion No, not planned Yes
Control columns (ex: checkboxes, actions) Rigid support Flexible support
Sticky columns No, not planned No
Likelyhood of additional feature support from EUI Not likely, simple tables are solved and work well Likely, data tables features are complex and potentially endless

@cchaos
Copy link
Contributor

cchaos commented Jul 20, 2020

Nice! I see a copy/paste happening soon into EUI docs of this comparison table... 😆

I updated the column headers to be explicit about that we are comparing EUI table components and not implementations of tables in Kibana.

@timroes timroes mentioned this issue Aug 6, 2020
29 tasks
@wylieconlon
Copy link
Contributor Author

The main conclusion I'm seeing here is that we should transition to DataGrid. Going to change this from a discussion to tech debt.

@wylieconlon wylieconlon changed the title [Lens] Discuss whether to replace basic table with EuiDataGrid [Lens] Replace basic table with EuiDataGrid Sep 15, 2020
@wylieconlon wylieconlon added technical debt Improvement of the software architecture and operational architecture and removed discuss labels Sep 15, 2020
@AlonaNadler
Copy link

Do we need to do it for Lens default?

@wylieconlon
Copy link
Contributor Author

@AlonaNadler My opinion is that this is not needed for Lens by default because this is an incremental improvement. The only table-related feature we have for Lens by default is the ability to sort the columns, and that's something that can be done with both tables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture
Projects
None yet
5 participants