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

Can we have a generic "button" component back? Or a pared down version? #1722

Closed
hross opened this issue May 4, 2017 · 7 comments · Fixed by #1733
Closed

Can we have a generic "button" component back? Or a pared down version? #1722

hross opened this issue May 4, 2017 · 7 comments · Fixed by #1733
Assignees

Comments

@hross
Copy link
Contributor

hross commented May 4, 2017

I know this was just switched over and I don't know the behind the scenes reason, but... it was convenient to have a component. Here is an example scenario where I would like to have it:

We have a page where the primary and default button change depending on what actions the user takes (or has taken) on the page. Example: I vote, and the primary/default buttons swap based on my vote.

The problem with having separate components in React is that when we re-render after the user action we lose focus. The only way to fix this issue with the current design (that I am aware of) is to track focus in another component, re-set focus in the DOM, etc, etc. It seems messy and inconvenient when I really just want a button that is primary vs. default.

@cschleiden
Copy link
Contributor

Isn't it weird for a user to switch a focused element while it's focused?

@hross
Copy link
Contributor Author

hross commented May 4, 2017

I tried this out with the exiting button swap and it doesn't work anyway, because it still swaps the buttons under the covers. Maybe it is a bit weird, but changing default behavior on the fly is definitely a valid case.. it would be as if you submitted a form and then the result came back up so you changed the UI.

I think in this case I am going to have to implement focus checking on the native elements. I will update this issue if that works for me. Maybe I can just add an example to the button page.

@hross
Copy link
Contributor Author

hross commented May 4, 2017

doing the focus check worked for me and it wasn't totally awful...

@hross hross self-assigned this May 4, 2017
@hross
Copy link
Contributor Author

hross commented May 4, 2017

I will take this and add an example when I get a chance to.

@dzearing
Copy link
Member

dzearing commented May 5, 2017

@hross We have this solved in knockout code with a utility that watches for this kind of thing. If focus goes to body for some period of time, and the previously focused element is no longer available, we can restore focus to something "close" to the previously removed thing. It would be nice to add utilities in fabric/utilities for managing focus restoration.

It's a general problem, I don't think really specific to the button. I think this would apply even if you were to say, change a input box to a text area or a dropdown into a list, or remove an item in a list, or programatically expand/contract a tree node where a child has focus.

I have thought about just collapsing Default/Primary button into PushButton, which takes a general "primary" flag. This would help your scenario, and I think aligns better with win32 components.

@hross
Copy link
Contributor Author

hross commented May 5, 2017

I like the idea of combining those two buttons. I thought similarly along those lines when I started doing this. All that said, the focus solution isn't terrible and as you said, we do need it elsewhere in some cases.

The global utility/framework thing is an interesting one. We don't have anything like that. If there were a utility in fabric that did that we would definitely drop it into our page if we could (we have signalr events that can cause elements to get zapped).

@cschleiden
Copy link
Contributor

Should this be a component similar to FocusZone? You wrap your focus-able components, maybe set a direction (select next item or previous item when focused item goes away), and it manages that for you?

@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants