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

IconButton: Recommended approach to showing toggle state #9941

Closed
1 task done
phallguy opened this issue Jan 18, 2018 · 11 comments
Closed
1 task done

IconButton: Recommended approach to showing toggle state #9941

phallguy opened this issue Jan 18, 2018 · 11 comments
Labels
component: icon button This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process

Comments

@phallguy
Copy link
Contributor

I'm working on a tool that allows certain states to be toggled from a toolbar (think bold/italic/etc). I'd like to show that certain states are enabled. I know the switch component makes this every easy but I'd like to mimic existing toolbar like functionality found in most WYSIWYGs.

What is the recommend approach for showing toggle state with an IconButton?

This may be outside the material spec but I would suspect something others would like do in their apps as they become more rich and rely more heavily on material-ui for all of the UX.

  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

I'd like some sort of 'on' or 'pressed' prop that results in corresponding visual representation on the IconButton.

Current Behavior

No props available. Which I think is by design but hopefully the use case is persuasive to reconsider.

Context

I hope to use this metaphor in a WYSIWYG editor as well as a few other toolbars within the app. I think a toggle state on a button more closely aligns to user's expectation of how a format toolbar should work than some sort of switch control.

Your Environment

Tech Version
Material-UI 1.0.0-beta.29
React 16
browser chrome
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 18, 2018

@phallguy I believe the relevant section in the specification is the following: https://material.io/guidelines/components/buttons.html#buttons-toggle-buttons. The 'on' state should simply be a matter of setting a color="primary" or color="secondary" property.

However, it seems that we don't follow the specification closely enough. The focus indicator ring should take the color of the icon into account. Right now, we don't.

@oliviertassinari oliviertassinari added design: material This is about Material Design, please involve a visual or UX designer in the process component: icon button This is the name of the generic UI component, not the React module! labels Jan 18, 2018
@phallguy
Copy link
Contributor Author

Ahh thanks for pointing me in the right direction. Didn't even think to check the buttons page - had 'toggle' on the brain! This will work for me. Thanks!

@phallguy
Copy link
Contributor Author

Having another look at it - the look I'm going after would be the "toggle buttons" shown in a group where it looks pressed.

@phallguy
Copy link
Contributor Author

I'm happy to submit a PR if this is a behavior the material team would like to include in the library.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 19, 2018

the look I'm going after would be the "toggle buttons" shown in a group where it looks pressed.

@phallguy We have had a primary effort in this direction with #7551. Unfortunately, it was never finished nor polished. Yes, it would be awesome to have it! Thanks for proposing your help. I would encourage you to:

@phallguy
Copy link
Contributor Author

Great, I think I can pull something simple together. Really love the work you guys are doing and would love to be able to give a bit back.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 20, 2018

@phallguy I have closed the issue. The IconButton better follow the specification now.
It would be good to have an issue for this new component. Have you made any progress? How do you want to call the new components? ButtonGroup, ButtonToggle, ButtonToggleGroup, etc.?

@phallguy
Copy link
Contributor Author

Haven't gotten started on it yet, was going to spend some time on it today.

I'm thinking as first step adding some sort of 'active' prop to ButtonBase so that each of the various buttons can be used in a toggle state and ensuring that the IconButton uses the current color for the focus ring. Then was going to start on the menu style toggle buttons. The Supported Comments doc calls them Toggle buttons (same as material UI) so I was going to go for ToggleButton and if needed ToggleButtonGroup. But happy to use ButtonToggle if you'd prefer.

I'll open a new issue for each of these and we can formalize requirements in those.

@mbrookes
Copy link
Member

ensuring that the IconButton uses the current color for the focus ring

@oliviertassinari fixed that today: #9967

ToggleButton sounds more natural to me. (IconButton, ToggleButton...)

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 20, 2018

@phallguy ToggleButton and ToggleButtonGroup sounds great to me 👍 .

ButtonBase contains as few styles as possible. It aims to be a building block for people who want to create a simple button. It contains a bunch of style reset and some focus/ripple logic. We will most likely simplify it further with #9526. I would rather not change the component if a simple alternative is possible.

@phallguy
Copy link
Contributor Author

phallguy commented Feb 2, 2018

Filed initial PR #10144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: icon button This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process
Projects
None yet
Development

No branches or pull requests

3 participants