-
Notifications
You must be signed in to change notification settings - Fork 7.6k
[Themes] Dialog Cancel button is broken or unnecessary #8390
Comments
@redmunds So, maybe this is just an issue with usability. But selecting themes isn't really related to the cancel/done button. The theme dropdown is what drives it. The rest of the settings are the ones that are tied to the buttons. So, the idea is to allow users switch themes without dismissing the dialog, which is rather cumbersome. Maybe if the user cancels out we revert the theme they selected? |
@MiguelCastillo I agree it's useful to immediately update the editor to quickly see how a Theme looks. If choosing a Theme were the only control in this dialog, then I would say just remove the Cancel button. But, since there are other controls, then I think that reverting the Theme (and all other settings) on Cancel is the way to go. |
+1 Sometimes apps will use a "Close" or "Done" button instead of "Cancel" to make it clear that changes made in the dialog aren't undoable... but IMHO that's sort of an anti-pattern. Much better to behave like other Brackets dialogs and allow backing out without making permanent changes. |
@peterflynn Yeah I think that canceling out of the dialog should revert the selected theme, for sure. I will put something together for this and submit a PR soon. |
FBNC to @redmunds |
Confirmed. Closing. |
When I change Current Theme, Brackets updates immediately. Then I click Cancel button and Dialog closes, but Theme does not revert to original. Not sure if Cancel button is broken, or it is unnecessary, but a change needs to be made one way or the other.
The text was updated successfully, but these errors were encountered: