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

[Discover] Selecting a number with a short value covers the field entirely #133739

Closed
Tracked by #7112
majagrubic opened this issue Jun 7, 2022 · 11 comments
Closed
Tracked by #7112
Labels
blocked EUI Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. upstream

Comments

@majagrubic
Copy link
Contributor

Kibana version:
8.3.0

Elasticsearch version:
8.3.0

Server OS version:

Browser version:
Chrome 101

Browser OS version:
Mac OSX

Original install method (e.g. download page, yum, from source, etc.):
Cloud

Describe the bug:
I believe this is a EUI bug, but given I don't have time for a detailed debugging, hope you can pass it on to the appropriate place.

When selecting a numeric field with a short value, the field is covered entirely by cell controls. This is confusing as I can't see the field value anymore.
Screenshot 2022-06-07 at 13 15 45

Steps to reproduce:

  1. Open Discover
  2. Create a numeric field in an existing data view
  3. Set the value of the field created in step 2 to one or two-digits
  4. Load the field in Discover
  5. Click on the cell
  6. Observe how the value is hidden by cell controls.

Expected behavior:
The field value should be visible.

@majagrubic majagrubic added Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. labels Jun 7, 2022
@elasticmachine
Copy link
Contributor

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

@kertal kertal added bug Fixes for quality problems that affect the customer experience :DataDiscovery/fix-it-week labels Jun 8, 2022
@jughosta
Copy link
Contributor

I think it would be better to address it in EUI. There is also a different behaviour which is supported in EUI but only for single line option.

Jun-14-2022 16-19-19

@jughosta
Copy link
Contributor

@andreadelrio @constancecchen Would it be possible to improve multi-line cells rendering in EUI grid when values are short?

@elasticmachine
Copy link
Contributor

Pinging @elastic/eui-design (EUI)

@kertal
Copy link
Member

kertal commented Jun 15, 2022

I'm removed this from our fix-it-week, labeled it upstream since the solution would be to improve it in EUI

@kertal kertal removed the bug Fixes for quality problems that affect the customer experience label Jun 15, 2022
@kertal kertal added the blocked label Jul 27, 2022
@kertal kertal added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Oct 27, 2022
@MichaelMarcialis
Copy link
Contributor

Due to the flexible nature of the data grid cells and their contents, I'm thinking that any design that has these cell action buttons sharing the space with the cell contents is going to be problematic. @majagrubic's and @jughosta's screenshots above shows how it can be problematic in situations where right-aligned cell contents can be covered in non-single-line instances. However, it's also problematic in situations when the user has narrowed their data grid column width (which potentially covers cell contents, truncates cell contents, or causes the action buttons to be cut off).

Image

I think the best path forward to address all these cases would be to change the placement of these cell action buttons to be displayed outside of the target cell's content area. For example, here's a quick rough mockup of what I'm thinking:

Image

We could also account for overly narrow columns by not forcing the actions below to adhere to the width of the target cell.

Image

In this way, the cell being focused/targeted will never having its own contents obscured by its related actions. Thoughts?

@davismcphee
Copy link
Contributor

@MichaelMarcialis I much prefer your proposal over the current cell actions! 🙌 I think this would address most if not all of the complaints with the current design.

Just two points of feedback:

  • In Discover and probably elsewhere in Kibana we sometimes display very tall cells, for example when viewing a large ES doc. Putting the cell actions below the cell would mean the user would have to scroll to the end of the cell in order to access them. Do you think this would be acceptable compared to the current experience, or should we consider positioning them above or beside the cell instead?
  • The current cell actions are a built-in part of EuiDataGrid and changes to them will likely need to be addressed directly in EUI. Do you think it would make sense to open a separate issue in the EUI repo to track this enhancement that we could link here?

@cee-chen
Copy link
Contributor

cee-chen commented Nov 2, 2023

Example of the "tall cell" problem Davis mentioned above: #133739

Definitely better than covering the entire text for sure 😅 but maybe top alignment for those cell heights makes more sense?

@cee-chen
Copy link
Contributor

cee-chen commented Nov 2, 2023

Sorry, hit enter too soon. To your second bullet point, here's the relevant EUI issue: elastic/eui#5828 (FWIW, I strongly prefer Michael's suggestion in here over Caroline's!). Up to you all if you'd prefer to close all the linked Kibana issues and consolidate them in one place

@MichaelMarcialis
Copy link
Contributor

@davismcphee, @cee-chen: Good call on the tall cell heights. I think moving the offset actions to the top makes sense given that possibility. We can probably also align the actions left or right depending on the cell contents as well (ensuring the actions are always in close proximity to the cell contents).

CleanShot 2023-11-03 at 10 32 15

CleanShot 2023-11-03 at 10 34 11

I'll copy/paste my notes from here into the EUI issue that Cee shared.

@cee-chen
Copy link
Contributor

cee-chen commented Jan 8, 2024

This should be resolved by #173569!

@cee-chen cee-chen closed this as completed Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked EUI Feature:Discover Discover Application impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. upstream
Projects
None yet
Development

No branches or pull requests

7 participants