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

[EuiDataGrid] Right-aligned clickable cell contents and cell actions are hard to use #5828

Closed
flash1293 opened this issue Apr 21, 2022 · 23 comments · Fixed by #7343
Closed
Assignees

Comments

@flash1293
Copy link
Contributor

If the content of a cell is both right aligned and clickable (e.g. a link), then the cell actions sliding in on hover make it hard to hit the link:
bait

A straightforward solution to improve the issue in practice in a lot of cases is to change the alignment to left, but in situations like the one from the recording above with narrow columns it's easy for the popovers to cover the thing the user wants to click even for left aligned text.

A more complex solution might be to show the cell actions below/to the right of the actual cell in case of clickable content so they don't cover the very thing in the focus of the user.

@snide
Copy link
Contributor

snide commented Apr 21, 2022

Honestly I think most of this is due to the animation. This becomes a bigger problem when there are more icons (like security) and then animation shifts more. I'd suggest leaving the delay, and removing the animation.

@flash1293
Copy link
Contributor Author

Not having an animation will make it slightly better, but at least for me I'm sure I will still hit the "filter out" button from time to time.

@cee-chen
Copy link
Contributor

I definitely get wanting to immediately click the link itself, but couldn't the user could always click the expand icon or the Enter key and click the cell link from there? Alternatively, for columns where this is a known issue, would setting a wider default width make sense? (No worries if the answer to both is no or a more graceful solution would be preferred)

@flash1293
Copy link
Contributor Author

click the expand icon or the Enter key and click the cell link from there

An additional click is a pretty high friction for such an interaction IMHO. Also (and that's a separate issue) it doesn't work in Discover because the link formatter is not applied in the popover.

for columns where this is a known issue, would setting a wider default width make sense

It doesn't help for right aligned texts, but it definitely improves the issue for left alignment (at the expense of a less dense table)

@cee-chen
Copy link
Contributor

Related: elastic/kibana#133739

@cee-chen
Copy link
Contributor

@miukimiu any thoughts or recommendations on how we could potentially move or redesign the cell actions hover/UX for the scenarios in this issue as well as in elastic/kibana#133739? Is displaying the actions outside the cell an option?

@kertal
Copy link
Member

kertal commented Jul 5, 2022

I also would a favor showing the actions outside the cell given there's not enough screenspace ... or to just show the expand icon for this case , icons would then be displayed in the cell expansion. Also had the same use case when trying out a grid @jughosta created. Wanted to click on a link in a cell, but that was a challenge since the cell actions didn't want me to click the link 😄

@cee-chen
Copy link
Contributor

elastic/kibana#99312 has a worse example of this. Just brainstorming, but it sounds like data grid needs to check when the cell is too small for the cell actions to display without blocking the majority of the content, and figure out some other way of displaying the cell actions. Maybe a popover?

@cchaos
Copy link
Contributor

cchaos commented Jul 27, 2022

I would just opt for a minimum width of that cell / column based on the number of cell actions + extra for content reveal.

@cee-chen
Copy link
Contributor

a minimum width of that cell

That's tricky, if the cell contains legitimately very long content that the user wants to hide, we'd essentially not be allowing users to resize column widths smaller than their content.

Maybe a static minimum would be helpful here, e.g. a minimum of 20px + width of cell actions?

@cchaos
Copy link
Contributor

cchaos commented Jul 27, 2022

Yeah sorry that's what I meant, the minimum width being based on the total width of the cell actions (plus some extra, probably a tiny bit more that 20px. It could even use ch (character) based length. Like min-width: ${cellActionsWidth} + 5ch (5 characters).

@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@jughosta
Copy link
Contributor

Yes, still valid

@github-actions
Copy link

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

@jughosta
Copy link
Contributor

Don't close please

@MichaelMarcialis
Copy link
Contributor

Copying thoughts from our discussions here: elastic/kibana#133739

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. It can be problematic in situations where right-aligned cell contents can be covered in non-single-line instances. 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

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 and above the target cell's content area. The action buttons should be aligned to the target cell's left or right side to match the alignment of the text within the cell. 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?

@cee-chen
Copy link
Contributor

cee-chen commented Nov 3, 2023

Love it - but do you mind posting a mockup / visual of what you anticipate the hover state looking like (i.e., when there's no blue border, which only appears on focus state)?

@cee-chen
Copy link
Contributor

cee-chen commented Nov 4, 2023

@MichaelMarcialis Not sure what you had in mind, but here's what I spiked out so far:

datagrid_cells

I basically made the background color on hover the same as the border color, to match the focus theme. It's not quite as convincing on borderless data grids however:

BTW, I also opinionatedly overrode (via !important css) any colors that consumers may have applied to the icon via EuiButtonIcon props. This is worth noting as it was previously something we allowed devs to customize (e.g. setting an accent icon) but no longer makes sense to do with this new design (just IMO). If you agree with making that choice, we can enforce this via types if necessary as a breaking change.

@cee-chen cee-chen self-assigned this Nov 5, 2023
@cee-chen
Copy link
Contributor

cee-chen commented Nov 5, 2023

@MichaelMarcialis Here's what I came up with for the hover state:

Essentially the focus ring now also shows on mouse hover and immediately turns blue on click/keyboard enter. The above comment/question about needing to override custom action colors still remains. You can try it out here: https://eui.elastic.co/pr_7343/#/tabular-content/data-grid

BTW, while working on this, I removed the previous hover delay, as actions no longer occlude cell content, and users having immediate access to cell actions feels better. I also took the time to spike out #5686 and aligned cell expansion popovers to the new cell actions - IMO, the new design looks and feels incredibly natural/delightful with it:

datagrid_cells

@MichaelMarcialis
Copy link
Contributor

Thanks, @cee-chen! Apologies for the delay, as I was out on PTO yesterday. Will review all these scenarios and see if I can provide you a comprehensive document with all the different states accounted for.

@MichaelMarcialis
Copy link
Contributor

Hey, @cee-chen. I put together my thoughts in a quick Figma document. I'll also include a screenshot here for visibility. My suggestions here are largely the same as you've shared above with some small differences, including:

  • Rather than the light gray for the hover border and button background colors, I suggest using a slightly darker gray (the same used for our text-colored buttons).
  • Rather than showing the action buttons immediately on hover, I think it still would be nice to have the actions appear after a delay before transitioning in. The inner cell border can continue to show with no delay on hover though.

Anyway, let me know if that makes sense or if you have any questions. Thanks!

Frame

@cee-chen
Copy link
Contributor

cee-chen commented Nov 16, 2023

Thanks Michael! Both items have been updated in #7343. LMK if https://eui.elastic.co/pr_7343/#/tabular-content/data-grid looks and animates how you would expect. If so, would appreciate you approving the PR!

One note on the animation delay - it makes the removed border-radius on the corner with the actions look a tad weird in the split second before the actions pop up. I doubt most would notice, but if it's bothersome, maybe we should consider doing away with the outline border-radius entirely?

@cee-chen
Copy link
Contributor

Closed by #7343

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants