Eui: [EuiDataGrid] Columns popover should not only show ids

Created on 5 May 2020  路  5Comments  路  Source: elastic/eui

Currently the popover to control visibility of columns and reodering them always shows the id property of that column. I think it would make more sense to show the display of that column or introduce at least a mode to configure that.

The id of a column can be something very technical, that the user otherwise will never see and thus cannot make a connection to the column they are seeing. e.g. in Discover the id could be an underlaying field name while we allow someone to give the column a custom header. Now they would always see the custom name in the column (e.g. "Current Status"), but in the popover they would always see "internal.stats.state_i900", which I think is not a good idea.

I understand that display can be used to render really rich controls inside the column header, why it might make sense not to always show display in the popover, but I think we should do one of the following solutions:

  • Add a flag to the data grid, allowing to configure whether id or display should be shown, so consumers can set it to display when they know it's just used to render alternative names; or/and
  • Give column descriptions a new property like displayAsText, which can only return a string and will be used in that popover dialog, so you can have a rich JSX display for the column header and still have a display name for the columns popover.
engineer data grid good first issue

All 5 comments

Give column descriptions a new property like displayAsText, which can only return a string and will be used in that popover dialog, so you can have a rich JSX display for the column header and still have a display name for the columns popover.

That one sounds best to me. We'll want to use the same value in the Sort fields popover, and it wouldn't surprise me if there's more places to re-use it in the future.

Should displayAsText must be a required prop so that it can replace id in the column_selector and Sort fields ?

I think it's ok for this to be optional, but this is a new enough component the breaking change wouldn't likely be too bad. Making it required would ensure consumers really think about their presentation, and has the benefit of being i18n translatable if need be.

I'd wait on @chandlerprall for his response, but that would be my reasoning for making is a required prop.

I'd leave it as optional to make getting started with the grid easier, and our docs can provide the full story for enhancing. Also, when the columns are displaying results from a query, I imagine it would be common to duplicate the id into the name field, which feels a bit burdensome.

I would like to work on this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cahna picture cahna  路  3Comments

roberto910907 picture roberto910907  路  3Comments

johnbarrierwilson picture johnbarrierwilson  路  3Comments

stacey-gammon picture stacey-gammon  路  4Comments

jen-huang picture jen-huang  路  4Comments