Skip to content
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

Updating GUI to use other types of notifications vs all pop ups #1517

Closed
JillieBeanSim opened this issue Oct 6, 2021 · 20 comments
Closed

Updating GUI to use other types of notifications vs all pop ups #1517

JillieBeanSim opened this issue Oct 6, 2021 · 20 comments
Assignees
Labels
Epic Technical Debt Includes Architecture, Code, Testing, Automation debt

Comments

@JillieBeanSim
Copy link
Contributor

JillieBeanSim commented Oct 6, 2021

Assessing other types of messages from #213, another one we would like to see gone is the retrieval message like the Get data set list submitted or the others from other trees. For those messages we also run the loading bar at the top of the appropriate view, I think just having the loading bar is enough for that action and would cut down on the amount of those pop ups that appear. Same thing when expanding a data set or folder.

I don't think we really need the opening file message when a file is being opened, or if we would like to keep it then it could also be put it in the status bar.

@JillieBeanSim JillieBeanSim added enhancement New feature or request Technical Debt Includes Architecture, Code, Testing, Automation debt and removed enhancement New feature or request labels Oct 6, 2021
@phaumer
Copy link
Member

phaumer commented Oct 28, 2021

We should follow the VS Code style guide for notifications: https://code.visualstudio.com/api/references/extension-guidelines#notifications

@rudyflores
Copy link
Contributor

Hi everyone! The following chart represents a proposal for updating the GUI to use proper notifications based on VS Code's style guide for notifications. In order to understand the terms referred to in the "Proposed UX" section of the chart go to VS Code's style guide for notifications, after reading it I encourage to check out the following:

Function/Message Current UX Proposed UX
"Get DS/USS/Job list command submitted" Pop-up message Progress bar message
"Opening DS/USS/Job" Pop-up message View notification
"Saving DS/USS/Job" Pop-up message View notification
"DS/USS/Job uploaded successfully" Pop-up message View notification
"Profile profileName was created" Pop-up message Pop-up notification
"Validating `ZOSMF/RSE" profile" Pop-up message Progress bar message
"No valid value for z/OS URL. Operation Cancelled" Pop-up message Status bar notification
"Profile was successfully updated" Pop-up message Status bar notification
"Profile profileName was deleted" Pop-up message Pop-up message

As for most error messages I believe it would be great to either handle them as View notifications or Error status bar items since this would allow less pop-up notifications that may be too distracting for the user as the style guides from VS Code mention.

@jellypuno
Copy link
Contributor

tagging @tomofbroadcom for awareness

@crawr
Copy link
Contributor

crawr commented Nov 9, 2021

The squad needs to look into this and respond with proposals/feedback. Discussion will take place on 16 November

@lauren-li
Copy link
Contributor

Thanks @rudyflores for taking the time to review the current notification UX and make a proposal. Here is my feedback on the proposed UX. I have bolded the functions/messages of interest:

  • "Get DS/USS/Job list command submitted": Progress bar message -> View notification (assuming "View notification" is just the progress bar in the view). This is a super-common operation, and probably the one whose pop-up messages (or pop-up progress bar messages?) bother users the most.
  • "Opening DS/USS/Job": Agree with View notification
  • "Saving DS/USS/Job": Agree with View notification
  • "DS/USS/Job uploaded successfully": Agree with View notification
  • "Profile profileName was created": Agree, assuming "Pop-up message" = "Pop-up notification"
  • "Validating `ZOSMF/RSE" profile": I'm not sure what difference is meant by "Pop-up message" versus "Progress bar message". (Actually, I thought the current UX is already "Progress bar message", and think it makes sense to keep it as-is).
  • "No valid value for z/OS URL. Operation Cancelled": Status bar notification -> Keep as Pop-up message (because this is a "special event")
  • "Profile was successfully updated": Status bar notification -> Keep as Pop-up message - I do not believe most users will be updating their profiles so frequently that having this as a pop-up will annoy them. However, I also don't feel too strongly about this if others prefer to change it to a status bar notification.
  • "Profile profileName was deleted": Agree to keep as Pop-up message

@lauren-li
Copy link
Contributor

For reference, here is my understanding of the different notification types:

  • Pop-up message/Pop-up notification: VS Code's pop-up that appears in the lower-right corner of its window:
    Screen Shot 2021-11-16 at 4 48 43 AM
  • Progress bar message: Basically a pop-up message/notification with a progress bar at the bottom of it. Goes away once the function has completed:
    ze-progressPopUp
  • View notification: A progress bar that shows underneath the view header, without any associated message text:
    ze-viewNotification
  • Status bar notification: A notification that appears at the bottom of the IDE window. Can either go away when the function completes, or hang around for longer. Overusing this may interfere/compete with other VS Code extensions' status bar messages.
    ze-statusBar

Bonus: For reference, there is also the dialog box, but I don't believe it is applicable to any of the functions/messages being considered as part of this issue:
Screen Shot 2021-11-16 at 5 05 32 AM

@tomofbroadcom
Copy link

@rudyflores

I reviewed it and also based on you explanation, I agree with the proposed way of standardisation. The VS Code guide is great.
I would just add one more thought to it:

  • Primarily focus only on errors and its wording and proposed solutions/actions leading to recovery
  • Focus on processes that takes more than expected time and then inform user about it, not sooner (this will vary for each process)
  • For processes that are expected to take some time the status bar progress seems quite inadequate, because it's hard to see it, since there is no change of colour and the status bar changes all the time... but ok
  • The rest doesn't need to have any notification of the result IF it's perceivable on the screen

Thank you! I appreciate this proposal.

@JillieBeanSim
Copy link
Contributor Author

I agree with @tomofbroadcom about focusing on error messages, especially when referring to pop up notifications as this will be easily seen by the user.
IMO the Profile name created could be a limited time status bar message, especially since the new profile name will also be added to the view upon success.
Same with Profile deleted pop up message. I noticed that the confirmation of Delete Profile is a Quick Pick where our other Deletes are now a VSC modal dialog box, would we want to standardize that as well?

@lauren-li
Copy link
Contributor

Thanks @tomofbroadcom for the UX pointers! The general advice is very helpful to consider.

~~

Same with Profile deleted pop up message. I noticed that the confirmation of Delete Profile is a Quick Pick where our other Deletes are now a VSC modal dialog box, would we want to standardize that as well?

@JillieBeanSim I think it makes sense to make the Delete Profile confirmation a modal dialog box to be consistent with our other Delete messages. I also agree with the logic you mentioned for having the Profile deleted message be a time-limited status bar message.

@jellypuno
Copy link
Contributor

jellypuno commented Nov 16, 2021

Research/Discussion: What operation should be cancellable or not? Can we combine multiple messages into one so it won't appear multiple times

Tagging: #1272

@rudyflores
Copy link
Contributor

Hey team! Just wanted to post a quick update on this issue thread regarding our latest discussion on notifications. The following table is an updated version of the previous table with the revised notification types to be used as discussed during scrum:

Function/Message Current UX Proposed UX
"Get DS/USS/Job list command submitted" Pop-up message View notification
"Opening DS/USS/Job" Pop-up message View notification
"Saving DS/USS/Job" Pop-up message View notification
"DS/USS/Job uploaded successfully" Pop-up message View notification
"Profile profileName was created" Pop-up message Status bar notification
"Validating `ZOSMF/RSE" profile" Pop-up message Progress bar message
"No valid value for z/OS URL. Operation Cancelled" Pop-up message Pop-up notification
"Profile was successfully updated" Pop-up message Status bar notification
"Profile profileName was deleted" Pop-up message Status bar notification

As for new notifications that were not addressed in the previous comment with the outdated table the following points have not been discussed or have not been agreed on which will be the approach for handling the notifications for the following:

  • Multi Selection
  • Research whether progress notification has a cancel button
  • Animations in the tree browser to view what is being saved

@rudyflores
Copy link
Contributor

rudyflores commented Nov 30, 2021

Hi everyone! Thanks for today's discussion on this issue and providing great feedback! The following table is the updated version with the changes we discussed as well as new additions which include whether these notifications will include animations or a cancel button:

Function/Message Current UX Proposed UX
"Get DS/USS/Job list command submitted" Pop-up message View notification animation
"Opening DS/USS/Job" Pop-up message View notification animation
"Saving DS/USS/Job" Pop-up message View notification animation
"DS/USS/Job uploaded successfully" Pop-up message View notification animation
"Profile profileName was created" Pop-up message Status bar notification
"Validating `ZOSMF/RSE" profile" Pop-up message Progress bar message with cancel button
"No valid value for z/OS URL. Operation Cancelled" Pop-up message Pop-up notification
"Profile was successfully updated" Pop-up message None
"Profile profileName was deleted" Pop-up message Status bar notification

Multi-selection and save all:

Function/Messsage Proposed UX
Multi-selection for add to favorites Progress bar message
Multi-selection for jobs when downloading spools Progress bar message with cancel button
Multi-selection for removing from favorites Progress bar message
Mutli-selection for hiding and deleting sessions Progress bar message
Multi-selection for members on job submissions Progress bar message with cancel button
Save all unsaved changes View notification animation

@crawr
Copy link
Contributor

crawr commented Dec 2, 2021

For "Profile profileName was deleted" , I think Status bar notification will not work for multi-selected items since our standard is to list all the deleted sessions. Perhaps the progress bar message should have the cancel button as well to be consistent with the other delete operations.

@lauren-li
Copy link
Contributor

lauren-li commented Dec 14, 2021

I believe in our scrum discussions, we had decided to show no notifications for the following, as users can easily verify their success from changes in the user interface:

  • "Profile profileName was created"
  • "Profile was successfully updated" Apologies @rudyflores, I think I was thinking about profile deletion when I added this item, except you had already changed the proposed UX for that to "none", so I accidentally put this instead. 😅

For the following item, I'm not sure if a view notification (animation) is possible, since the message makes it sound like it's something that appears once the action is complete, rather than during the request. We may want to double-check the code for this one:

  • "DS/USS/Job uploaded successfully"

@lauren-li
Copy link
Contributor

@rudyflores The most updated table in your #1517 (comment) looks good to me now. Thank you for your investigation on this!

@JillieBeanSim
Copy link
Contributor Author

In scrum today I mentioned a new location to hold all of the UI/UX code. I created this class to hold all UI/GUI actions like input boxes, dialog boxes, pop up messages, and so on to keep VS Code user interaction code separate from logic code when I added the Theia fix. I believe it will also help us if an issue arises like the last one with the Theia UI and fixes can be done in a single location instead of having to search through all of the code to find each instance. What do you think?
https://github.com/zowe/vscode-extension-for-zowe/blob/master/packages/zowe-explorer/src/shared/ui-views.ts

@lauren-li
Copy link
Contributor

In scrum today I mentioned a new location to hold all of the UI/UX code. I created this class to hold all UI/GUI actions like input boxes, dialog boxes, pop up messages, and so on to keep VS Code user interaction code separate from logic code when I added the Theia fix. I believe it will also help us if an issue arises like the last one with the Theia UI and fixes can be done in a single location instead of having to search through all of the code to find each instance. What do you think? https://github.com/zowe/vscode-extension-for-zowe/blob/master/packages/zowe-explorer/src/shared/ui-views.ts

I'm not sure if this question was for me, but I think it's a good idea! 😄

@JillieBeanSim
Copy link
Contributor Author

It's for all to weigh in on. It will be easier to refactor while working on this issue too piece by piece.

@rudyflores
Copy link
Contributor

In scrum today I mentioned a new location to hold all of the UI/UX code. I created this class to hold all UI/GUI actions like input boxes, dialog boxes, pop up messages, and so on to keep VS Code user interaction code separate from logic code when I added the Theia fix. I believe it will also help us if an issue arises like the last one with the Theia UI and fixes can be done in a single location instead of having to search through all of the code to find each instance. What do you think? https://github.com/zowe/vscode-extension-for-zowe/blob/master/packages/zowe-explorer/src/shared/ui-views.ts

I love this idea! it would help for future maintenance and also have a global object that can be instantiated whenever we need to utilize the UI elements within the extension!

@JamesBauman JamesBauman assigned JamesBauman and unassigned rudyflores Feb 7, 2022
@zFernand0 zFernand0 removed the 22PI1 label May 5, 2022
@JillieBeanSim JillieBeanSim mentioned this issue Oct 11, 2022
33 tasks
@zFernand0 zFernand0 assigned anaxceron and unassigned JamesBauman Oct 11, 2022
@traeok traeok linked a pull request Jan 5, 2023 that will close this issue
16 tasks
@traeok traeok removed a link to a pull request Jan 5, 2023
16 tasks
@JillieBeanSim JillieBeanSim mentioned this issue Jan 20, 2023
31 tasks
@JillieBeanSim JillieBeanSim mentioned this issue Mar 27, 2023
27 tasks
@JillieBeanSim
Copy link
Contributor Author

closing Epic, all stories are completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Technical Debt Includes Architecture, Code, Testing, Automation debt
Projects
None yet
Development

No branches or pull requests

10 participants