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

Select: Overflow ellipsis and control over multi value wrapping #76405

Merged
merged 9 commits into from
Nov 14, 2023

Conversation

torkelo
Copy link
Member

@torkelo torkelo commented Oct 12, 2023

We are using Select & MultiSelect for variable value selection in scenes (so in app plugins and new core dashboards).

grafana/scenes#399

  • Overflow behavior (ellipsis) The "maxVisibleValues" is a not a good solution I think as it does not take into account the width of each value. Need to figure out something that is more adaptive / dynamic to value width.
  • Option to control No wrapping of values when not opened.

Done

  • New no wrapping option (noMultiValueWrap). Disables wrapping when the menu is closed. Only relevant for MultiSelect as normal Select never wraps
  • Add text truncation to multi-value items that do not fit.

Before:

A long value:
image

Many values that cause wrapping
image

After:
Single long value and noMultiValueWrap=true
Screenshot from 2023-11-06 12-43-16

maxVisibleItems = 4 and noMultiValueWrap=true
Screenshot from 2023-11-06 12-46-09

Unrelated fixes

  • Trying to reduce the number of new components that are created on every render (lots of component functions are created on every render and passed to react-select via components). I think this is mainly due to it makes typing easier, and there is no need to pass custom props via selectProps.
  • Typing is a nightmare, I am trying to improve things but it feels like you spend 99% time on typescript issues than anything else. Need help from someone who has time to dig into select typing in a different PR preferably.

Copy link
Contributor

github-actions bot commented Nov 6, 2023

⚠️   Possible breaking changes

(Open the links below in a new tab to go to the correct steps)

grafana-ui has possible breaking changes (more info)

Console output
Read our guideline

@github-actions github-actions bot added the levitate breaking change A label indicating a breaking change and assigned by Levitate. label Nov 6, 2023
@torkelo torkelo marked this pull request as ready for review November 6, 2023 11:51
@torkelo torkelo requested review from grafanabot and a team as code owners November 6, 2023 11:51
@torkelo torkelo added add to changelog no-backport Skip backport of PR labels Nov 6, 2023
@torkelo torkelo changed the title Select: Better overflow and wrapping behavior and control Select: Better overflow and wrapping behavior Nov 9, 2023
@torkelo torkelo changed the title Select: Better overflow and wrapping behavior Select: Better overflow and control over multi value wrapping Nov 9, 2023
@torkelo torkelo changed the title Select: Better overflow and control over multi value wrapping Select: Overflow ellipsis and control over multi value wrapping Nov 9, 2023
Copy link
Contributor

@Clarity-89 Clarity-89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good in general, just wonder if the whole truncate behavior should be configurable via a prop? I assume there could be instances where the users of the component would not want to truncate the values for their own reasons and considering that this is the most widely used component we have, the number of such cases could be non-trivial.

packages/grafana-ui/src/components/Select/SelectBase.tsx Outdated Show resolved Hide resolved
packages/grafana-ui/src/components/Select/SelectBase.tsx Outdated Show resolved Hide resolved
@torkelo
Copy link
Member Author

torkelo commented Nov 13, 2023

@Clarity-89 The truncation only kicks in when the new option "noMultiValueWrap" is true. The non-multi-value select already truncates long values that do not fit, we have no option for that there so why have it for MultiSelect? I think this is a safe new behavior that should not change any current multi-selects (As they will continue to wrap and hence show no truncation )

@torkelo torkelo merged commit 867ff52 into main Nov 14, 2023
18 of 19 checks passed
@torkelo torkelo deleted the select-improvements branch November 14, 2023 07:29
@torkelo torkelo modified the milestones: 10.2.x, 10.3.x Nov 14, 2023
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add to changelog area/frontend levitate breaking change A label indicating a breaking change and assigned by Levitate. no-backport Skip backport of PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants