Allow suppressing UI blocking dialogs #2546
Conversation
plugin/tooling.py
Outdated
| sublime.error_message("The clipboard content must be a URL to a package.json file.") | ||
| msg = "The clipboard content must be a URL to a package.json file." | ||
| status = "Clipboard must be a URL to package.json" | ||
| notify_err(msg, status) |
There was a problem hiding this comment.
I honestly don't think we are gonna accept any of these changes (not in this form). It doesn't make sense to just blindly change all instance of modals. Especially in this file it's a case of explicit user action and it makes sense to report the error in the dialog (the user won't be surprised by it) instead of having to dig for more info in the console or potentially miss the status field message.
There was a problem hiding this comment.
The users to whom it makes sense to see these kind of dialogs or who worry about missing a status field message can simply continue to use the defaults. Or they can set the regular dialog suppression setting, but leave the error setting on.
So this setting is simply not for them, but for the other group who have never had an issue of missing a status field message (by the way, you could add ❗ symbol if you think this is a very important message, that's harder to miss), don't see the point of a blocking modal for a simple error message. For them these bad interface design patterns don't make sense because they do not reflect the importance of the messages.
explicit user action and it makes sense to report the error in the dialog (the user won't be surprised by it)
The issue here isn't surprise, but, requiring additional user action for no good reason.
What's your alternative - group/code all the messages and ask the user to provide a list of those that should be suppressed like in lints?
There was a problem hiding this comment.
In this case the error is a direct response to user action so it's fine to present a dialog that can be dismissed easily with a single button press.
If you don't like dialogs so much then maybe just create a plugin that overrides sublime.error_message (if that's possible but I think it should)? It's not sensible to expect every package to introduce an option to switch between error dialogs and status field messages.
For some cases I would also prefer to not have dialogs but as discussed in related conversation, there is not necessarily a better way due to ST's limitations.
There was a problem hiding this comment.
In this case the error is a direct response to user action
Showing a status message is also a direct response
so it's fine to present a dialog that can be dismissed easily with a single button press.
It's not fine, you need to provide value to the user to demand action from every single user (this is an optional config, disabled by default, so most of the users will continue to suffer through needless actions the way you want them), and in this case you don't - this is simply not an important error message.
Also for some of these cases a blocking message prevents a much better UI: for example, in the next error that appears after theparse VSCode package JSON action instead of a blocking URL must end with 'package.json'" it'd be better to show the same input field again with the old user value and show a non-blocking error message (e.g., in the status bar or maybe in some popup) so you don't lose valuable user input data and allow for the next user action to be error correction instead of UI unblocking
If you don't like dialogs so much then maybe just create a plugin that overrides sublime.error_message (if that's possible but I think it should)?
I don't like needless user actionn, not dialogs, so I don't want to override all the error messages, just the ones that aren't important. None in your plugin are, so I'd be fine with overriding all of them.
It's not sensible to expect every package to introduce an option to switch between error dialogs and status field messages.
I don't expect that, there are plenty of plugins that don't show dialogs, or use less intrusive notifications, and this is a PR for a single package optionally supressing unimportant error messages, not for Sublime core changing everything
|
Here is my opinion:
|
Yes, should be possible. Just load settings via |
|
Updated the PR per the following:
switched to suppress by default
Left the setting given that it's suppressed by default, also re the extra button: Sublime still hasn't learned to properly roundtrip user configs, and this button would change files in the User folder, thus breaking subtle formatting, so I'd just leave it in the currently less convenient format
Added a shorter text, otherwise it might not fit
switched
They address @rchl concern of users not noticing notifications, |
I do think that there will be a good portion of users that will not notice something showing up in the status field for a few seconds. Or if they do notice something changing, they might not realize what has changed (if the status field is busy). So as I said, I don't think that status messages are a good solution in general. I suppose when user is doing an explicit action then he/she might be more looking for some feedback/result and might pay more attention. If the user doesn't expect anything then it might be easy to miss status change completely. |
|
After checking the manually shortened messages, there is actually no ❗ left, only in the short template when the only thing you see is
Have you actually tried it? It's harder to miss colored ❗ motion in your vision periphery
By the way, for very important messages you could also use these popups, that's in user's focus (at the cursor) (and of course it could fit the whole message) 
|
eugenesvk
force-pushed
the
fr-no-modal
branch
2 times, most recently
from
86acd06 to
88f4c62
Compare
✅ Deploy Preview for sublime-lsp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
eugenesvk
force-pushed
the
fr-no-modal
branch
3 times, most recently
from
87c86f4 to
6bbced9
Compare
show became suppress, so dialogs reversed with status messages
fallback sublime functions
with an extra severe argument to downgrade to a non-error modal dialog if needed
eugenesvk
force-pushed
the
fr-no-modal
branch
from
b0ec6a6 to
546a8df
Compare
| @@ -449,6 +449,11 @@ | |||
| "default": false, | |||
| "markdownDescription": "Show errors and warnings count in the status bar." | |||
| }, | |||
| "suppress_error_dialogs": { | |||
| "type": "boolean", | |||
| "default": false, | |||
There was a problem hiding this comment.
This is true in the settings file.
There was a problem hiding this comment.
I would just print the messages to the status bar and console.
So let's revert it back to true then here and in the settings
There was a problem hiding this comment.
Yeah my intention was to fix this line, and not to change the value in the settings. But then I would think that probably nobody will change this setting anyway, so I would just leave it out. We can't add a new setting for each tiny aspect of this package's behavior. I don't expect that people will care very much when the error dialogs went away and I don't think they will complain about that. I recently also removed another of these dialogs in #2528 (which is not covered in this PR) and I think it is the right thing to do. Though if the other contributors think differently, I'd be okay if my opinion gets overruled. And I would let @rchl decide whether the error messages in tooling.py should keep their dialog windows or not, because iirc he implemented that (and as I already wrote, this troubleshooting command is not a common action during editing anyway).
|
I thought about this a bit and I would still vote to not add a new setting for this. I think these messages don't appear very often and for such a small matter this package should decide what the best UI/UX is. So I would just print the messages to the status bar and console. Perhaps as a compromise the error messages from the tooling / troubleshoot command should still keep the error dialog. To be honest, I've never used that command and probably most users haven't either. Another idea for (all) console logs from LSP would be to add a timestamp, similar to the server log panel. Then it would be easier to see which logs are relevant if there is an issue. But that could be added separately. |
But user configuration is that compromise. I, for one, would never want to see these as blocking either, showing informational error messages doesn't warrant this. And this package then can define whatever compromising defaults you want, it won't affect others permanently Besides this goes back to the 2-tier error message system that was just removed, where you had the flexibility of suppressing dialogs of only 1 type, both at the level of the plugin, and individual user |
Add 2 user configuration options that would allow to suppress the attention-demanding UI blocking dialogs, instead redirecting warnings/errors to a status message and the full console message
Simple way to close #2545 while preserving the current defaults
Though not all modals are affected since some are invoked by the lsp_utils dependency
Question: is it possible to make that dependency respect config options from this plugin? Or do dependencies "by design" ignore their clients?