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

[OneDiscover] Add new switch to Show row stripes #188063

Closed
Tracked by #191570
kertal opened this issue Jul 11, 2024 · 8 comments
Closed
Tracked by #191570

[OneDiscover] Add new switch to Show row stripes #188063

kertal opened this issue Jul 11, 2024 · 8 comments
Assignees
Labels
Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Project:OneDiscover Enrich Discover with contextual awareness Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@kertal
Copy link
Member

kertal commented Jul 11, 2024

Describe the feature:

It should be possible to configure the UnifiedDataTable in Discover to show / hide row stripes.

Mock:
Context-Aware_UX_For_Logs___Version_5__In_Progress__–_Figma

Notes:

  • Similar to other data grid settings, the row stripes config should be synced to the URL app state, persisted to local storage (to persist the latest setting), and persisted to saved searches.
  • There should be no change to the default setting of showing row stripes, which is active
@kertal kertal added Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Project:OneDiscover Enrich Discover with contextual awareness labels Jul 11, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@davismcphee
Copy link
Contributor

@MichaelMarcialis The first time I saw this new option was in the One Discover mockups, and I'm just curious why we decided to add it? Was it something raised by product? Mainly thinking if this is really going to be a valuable option to offer to users.

@kertal
Copy link
Member Author

kertal commented Jul 19, 2024

feels like this could be a next for @lukasolson since it's related to the density configuration of the data table.
No urgency of course, but maybe @davismcphee @MichaelMarcialis maybe we could clarify the context why this was added, it's also related to one older issue, allowing to remove the tokens of the headers #173155
These are small enhancements, that are all valid IMO. I'd personally tend to remove the tokens of the headers, as they take a bit of width of the table header text, when having much columns. I don't have a use case for removing 🦓 . I like 🦓 ... but of couse there might be people that prefer 🐴 .

@davismcphee
Copy link
Contributor

Yeah I'm not necessarily against doing this, mainly just looking to understand why we decided to add it and if it's a necessary option (similar to the header tokens one). I think we should be careful about which options we add to the popover to avoid overwhelming users / crowding the UI, and because each option we add means an additional property to manage on the saved search model.

@MichaelMarcialis
Copy link
Contributor

MichaelMarcialis commented Jul 22, 2024

@MichaelMarcialis The first time I saw this new option was in the One Discover mockups, and I'm just curious why we decided to add it? Was it something raised by product? Mainly thinking if this is really going to be a valuable option to offer to users.

Good question, @davismcphee. This was originally proposed as part of the Discover design refresh that @andreadelrio and I collaborated on, driven by feedback from Alexandros Batsakis. I only continued to include it for the One Discover context-aware logs mockups because I was assuming that was still planned to be implemented at some point. If there was a compelling reason to exclude it though, I'd be happy to have that conversation. It's a nice-to-have in my mind, but not something I'd consider a critical requirement. As for value to users, I know some users (including myself) find it to be easier to parse table rows with that sort of styling, rather than using borders alone.

@davismcphee
Copy link
Contributor

As for value to users, I know some users (including myself) find it to be easier to parse table rows with that sort of styling, rather than using borders alone.

Oh I fully agree with this, the row stripes are great for UX IMO, and we have them enabled by default in Discover currently. I just don't really think we need a toggle to turn them off since I imagine they help most users. So my ask is basically are we ok to keep row stripes enabled all the time and not offer a toggle to turn them off?

@kertal
Copy link
Member Author

kertal commented Jul 23, 2024

+1 for row stripes. I'd consider to keep the default, and consider having a switch to turn them off, if there's significant user feedback for having such a switch

@kertal
Copy link
Member Author

kertal commented Jul 30, 2024

Closing this as currently not planned, we can reopen any time

@kertal kertal closed this as not planned Won't fix, can't repro, duplicate, stale Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Project:OneDiscover Enrich Discover with contextual awareness Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants