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

Add toolbar button styles #4730

Open
ryankeairns opened this issue Apr 19, 2021 · 11 comments
Open

Add toolbar button styles #4730

ryankeairns opened this issue Apr 19, 2021 · 11 comments
Labels
feature request ⚠️ needs spec Should be groomed by the EUI team every week to ensure a spec is added Stale

Comments

@ryankeairns
Copy link
Contributor

Perhaps this could instead be a discussion topic, but Lens and the presentation toolbar (Dashboard, Canvas) are using the same toolbar button styles which smells like a possible EUI addition (e.g. new button group variation?).

They've been operating with separate sets of styles and, as of this writing, are having to make simultaneous adjustments due to the Amsterdam switch in order to gain more contrast with the underlying background color.

The latest design proposal is here (adding a border):

Screenshot 2021-04-15 at 14 24 37

@ryankeairns ryankeairns added the ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible label Apr 19, 2021
@miukimiu
Copy link
Contributor

Thanks @ryankeairns,

For sharing this. The Maps app also has toolbar buttons.

Screenshot 2021-04-19 at 14 53 06

I think in EUI we could definitely create a toolbar button and toolbar button groups to create some consistency across all apps.

@chandlerprall
Copy link
Contributor

Related: ARIA toolbar proposal #2405

@cchaos cchaos added assign:designer and removed ⚠️ needs validation For bugs that need confirmation as to whether they're reproducible labels Jun 9, 2021
@github-actions
Copy link

github-actions bot commented Dec 7, 2021

👋 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.

@cchaos cchaos added skip-stale-check ⚠️ needs spec Should be groomed by the EUI team every week to ensure a spec is added and removed stale-issue labels Dec 7, 2021
@cchaos
Copy link
Contributor

cchaos commented Dec 7, 2021

This issue is still valid, but it would be helpful if the EUI team had a more thorough design spec to go off of. @ryankeairns Is there someone on your team who is familiar with all the states/configurations that can build this out in Figma?

@ryankeairns
Copy link
Contributor Author

@cchaos yep, @MichaelMarcialis can assist with that although it won't be real soon. Let's consider that a next step and he'll get to it as his workload allows.

@daveyholler
Copy link
Contributor

@MichaelMarcialis and @ryankeairns - do y'all still wanna get this implemented in EUI?

@MichaelMarcialis
Copy link
Contributor

From my perspective, I believe it still makes sense to support this at the EUI level. The current landscape is a bit all over the place:

  • Lens uses the ToolbarButton shared component in Kibana.
  • Dashboard and Canvas use a mix of a deprecated QuickButtonGroup component alongside EuiButton components with style overrides.
  • Maps also uses its own set of toolbar button markup and styles.

In any case, either a new component or enhancement to the existing button components at the EUI level would likely help provide better consistency across the apps and help us retire some of these redundant, bespoke solutions.

@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.

@MichaelMarcialis
Copy link
Contributor

I believe this is still a relevant issue, and I'm seeing that I probably owe ya'll some additional spec details. I'll put it on my to-do list to write something up as soon as I'm able.

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.

@MichaelMarcialis
Copy link
Contributor

Still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request ⚠️ needs spec Should be groomed by the EUI team every week to ensure a spec is added Stale
Projects
None yet
Development

No branches or pull requests

7 participants