-
Notifications
You must be signed in to change notification settings - Fork 559
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
"Primary" and destructive action buttons #861
Comments
We should also consider keyboarding experience as well. a Primary action could be wired up to Enter, and desstructive could be Esc |
Here is another suggestion for naming the property: Other than that spec looks fine. One thing that isn't mentioned is whether several actions can have a "positive" or "destructive" emphasis. My opinion is we should simply allow it. |
where should the accent color and the destructive button color be taken from? I've seen color configurations for containers but there's no default color configurations for a general case |
After some discussion, I think that the default behaviors should be:
Changing the background on both could lead to both types of actions having the same or similar background color and causing confusion. I'm using Red as my accent color on Windows. |
We've been discussing this feature and I wanted to propose an extensibility point that hosts could support. If the card author specifies another sentiment value than the ones specified here, the renderers would still try to apply the sentiment as a style to the action. This would allow for a host to easily support more sentiments through an efficient extensibility model. For example, in the UWP renderer, the host can pass a resource dictionary with styles. Having The same thing can be applied to the JavaScript renderer with CSS classes. Thoughts? |
@khouzam I don't mind doing that. The way I see it, there would be a way in HostConfig to declare supported custom sentiment names:
At runtime, the renderer would check that a specified sentiment name in an action is declared in HostConfig; if so, it would apply the corresponding style (in HTML, simply use the matching CSS class) and if not it would use the default sentiment style. Is that what you have in mind? |
DECISION: For the action “sentiment” property, consensus was we should rename the property to “style”. Enum values remain the same (but the enum itself should be renamed to ActionStyle), and the values in it are different from ContainerStyle
|
✔️ Android (#1970)
✔️ iOS (#1971)
✔️ TS (#2435)
✔️ UWP (#2227)
Solves requests
Solution
Add a style property to all actions. The Style enumeration will initially support only three values, but we could add more styles in the future.
Schema
New property on
Action
Other property name ideas...
Concerns around
style
?style
seems like the best choice.ActionStyle enum
Research on enum value names...
This is different from choosing the default action, since sometimes you want the destructive action (like deleting) to be the default action.
Host Config
None. There's already no host config for specifying background colors of actions today. This should all be done using native styling.
Down-level impact
For authors: Low. Actions will still appear, just not with the style behavior.
For hosts: Breaking change. If host is currently natively styling buttons, and now a new primary button comes in, the new button has a new styling name key. Therefore, the host's current styling won't be applied. Hosts need to update their native styling so that when they receive a style, the button at least looks like a default button.
Renderer Requirements
Action
, but an emphasized button might get a style name ofAction.Positive
(or however the native platform styling naming convention works)Auto-generated task status
The text was updated successfully, but these errors were encountered: