Arvados

developer-run-tests-services-workbench2: #678

21313-share-dialog-warning @ d7cf4c1ec30c7f90f38a9f34c14547c94c081b59

• ✅ All agreed upon points are implemented / addressed.
• ✅ Anything not implemented (discovered or discussed during work) has a follow-up story.
• ✅ Code is tested and passing, both automated and manual, what manual testing was done is described
• ✅ Documentation has been updated.
  n/a
• ✅ Behaves appropriately at the intended scale (describe intended scale).
• ✅ Considered backwards and forwards compatibility issues between client and server.
• ✅ Follows our coding standards and GUI style guidelines.

Notes:
When typing in the participants field, the autosuggest populates a list of relevant suggestions. I think it's very obvious to the user that simply typing "John Doe" and hitting 'close' will not save any information. If the user hits Enter after typing the name, the first suggested name will be selected.

I changed it so that if there is a selected name (from the autosuggest list), the "CLOSE" button is disabled and a warning shows up explaining that the plus (+) button must be clicked to save the participant name. In this state, the only way to proceed is to either click the plus button to save the name or remove the name from the input field, so I changed the behavior to allow for background clicking to close the dialog. At that point, I think it is obvious to the user that nothing has been saved and it would be frustrating to the user to have no obvious way to close a dialog that they no longer want to interact with.

21313-share-dialog-warning @ d7cf4c1ec30c7f90f38a9f34c14547c94c081b59

I believe the (+) button was a response to older behavior where the button behavior was "Save and close" which was annoying if you wanted to make multiple edits to sharing. But the positioning is arguably awkward.

• Brainstorming: instead of disabling the Close button, could we swap between "Close" and "Save" depending on the state? Or have both buttons but "Save" is the one in the lowermost right corner because that's what people will go to first.
• So maybe the (+) button goes away so we don't have two save buttons
• Then we'd adjust the message: "Click Save to share"
• I don't think we want clicking outside the dialog box to close it when unsaved. It's too easy to miss the "save" button and click outside the box by accident and lose whatever you were doing. The ESC button also dismisses the dialog, I think we should keep that, because it is more intentional.
• Could you also align the horizontal lines under "Users" and "Authorization", it's a bit painful to look at.

developer-run-tests-services-workbench2: #720

21313-share-dialog-warning @ 4a1e25b2df1817cb3bfa153076efd1a98ed802ff

• Brainstorming: instead of disabling the Close button, could we swap between "Close" and "Save" depending on the state? Or have both buttons but "Save" is the one in the lowermost right corner because that's what people will go to first.
• So maybe the (+) button goes away so we don't have two save buttons

I tried the first approach, but having the single button switch between states was jarring and unpleasant imo. I opted for two buttons, each disabling on the 'saveEnabled' prop.

• Then we'd adjust the message: "Click Save to share"

Because of the buttons disabling, it's pretty obvious to the user what they need to do to proceed. I removed the error message entirely and it looks much cleaner.

• I don't think we want clicking outside the dialog box...

Done, now the only way to close the dialog when a user is selected but not saved is to hit ESC

• Could you also align the horizontal lines under "Users" and "Authorization", it's a bit painful to look at.

I fiddled with this for a while before realizing that the Material UI components actually render differently on Chrome and Firefox, meaning I would have to find a compromise between the two. I opted to just remove the underline from the Select component, making it match the other Selects in the dialog. It's still uneven, but basically unnoticeable now.

I also took the opportunity to re-style the grid and the components, because none of them really aligned before.

developer-run-tests-services-workbench2: #727

21313-share-dialog-warning @ 495ee6b835dea5f053cf95bef2dc9d61d0324774

Done and done

Lisa Knox wrote in #note-17:

developer-run-tests-services-workbench2: #727

21313-share-dialog-warning @ 495ee6b835dea5f053cf95bef2dc9d61d0324774

Done and done

This LGTM