Remove on-close events from json-config-editor#343
Remove on-close events from json-config-editor#343williamsyang-work wants to merge 1 commit intoeclipse-cdt-cloud:masterfrom
Conversation
efc74e3 to
045d887
Compare
|
Looks good generally. Once minor comment to address and I'll approve. |
| ); | ||
|
|
||
| if (submit === 'Yes') { | ||
| if (this.userClickedSubmit) { |
There was a problem hiding this comment.
The implementation works as described. However, I wonder if we should open a confirmation dialog before closing the config editor so that the user can cancel to not lose the configuration accidentally? The implementation could be similar to when closing a code editor with changes that have not been saved. WDYT @williamsyang-work ?
There was a problem hiding this comment.
During the original implementation of the customization UI I tried implementing that and wasn't able to. I think that stopping the closing of a file is one of those things that vscode reserves for itself and doesn't give extension developers. But I would need to verify that.
There are some other bugs we've found that I think should be addressed first. We can scope this in and attempt an implementation later?
There was a problem hiding this comment.
Ok. Thanks for the explanation. Too bad that the vscode APIs seem to be too restrictive. Still worth investigating a bit more to see if we manage to achieve this. We can scope this in later after your other bugfixes.
There was a problem hiding this comment.
I tested the config editor feature a bit more and noticed a few UX issues (without your PR). For example,
- when creating a config, the editor opens. The editor content is not saved at that moment. The name in header of the config editor is cursive. When I open a file from the file explorer, the config editor content is replaced and I get the submit configuration toast message is shown. With your PR, the editor for config would just be gone.
- when creating a config, the editor opens. Now I change the config. The editor content is now saved in a temporary file and the name in the header of the config editor is not cursive indicating a permanent opened editor. When I open a file from the file explorer, the new file opens and the name of the config editor is strikethrough (I guess the temporary file is deleted?) Then the submit configuration toast message is shown. With your PR, the editor for config would just be gone.
Both cases need some UX improvements. Are those cases part of the bugfixes that you're working on?
dd9ea97 to
4305b21
Compare
This will force the user to use the button commands in the nav bar. Signed-off-by: Will Yang <william.yang@ericsson.com>
4305b21 to
8543ef3
Compare
What it does
Removes the prompt for config submission if the user closes the file. We found it was counter-intuitive UI. This will force the user to use the button commands in the nav bar.
How to test
Close the custom analysis json config file. No prompts should show up!
Follow-ups
Really just continued UI/UX improvements for custom analysis.
Review checklist