-
Notifications
You must be signed in to change notification settings - Fork 197
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
[invokers] Should invokers toggling a dialog close or cancel? #937
Comments
For me it would make sense for cancel buttons to cancel and submit buttons to close. Given we can't know the intent for toggles I think close is the better approach. I do think a cancel action is useful separate to the toggle ones though. |
by cancel you mean button/input type=reset? what would button type=button do? Edit: the call today clarified my question, and i was mistakenly thinking that input type=reset also closed dialogs. |
The Open UI Community Group just discussed
The full IRC log of that discussion<jarhar> keithamus: the close button or default/auto behavior for an invoke button inside the dialog<bkardell_> close <jarhar> keithamus: could be toggle togglemodal or close <jarhar> keithamus: should it fire the cancel event? <bkardell_> it should not fire *a* cancel event <jarhar> masonf: the auto action is going to be togglemodal, so if the modal is already open its close <keithamus> Proposed resolution: closing a dialog from an invoker will not dispatch a cancel event <Luke> +1 <jarhar> +1 <keithamus> RESOLVED: closing a dialog from an invoker will not dispatch a cancel event |
There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label. |
Right now the explainer for invokers suggests that any of the toggle actions should cancel the dialog.
The cancel event is fired when a CloseWatcher closes a dialog, it currently doesn't fire for buttons, so having these buttons cancel the dialog would be somewhat new behaviour.
My vote is that they should close instead of cancel, but I'd like to hear others' thoughts.
/cc @lukewarlow @mfreed7 @scottaohara @domenic
The text was updated successfully, but these errors were encountered: