#31608 closed enhancement (fixed)
Shiny Updates: fine-tune UI for filesystem credentials request modal
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.2 | Priority: | normal |
Severity: | normal | Version: | 4.2 |
Component: | Upgrade/Install | Keywords: | has-patch ui-feedback needs-testing |
Focuses: | ui, accessibility, javascript | Cc: |
Description
The filesystem credentials request when updating or installing a plugin has been merged.
@melchoyce has previously drafted a mockup for a filesystem credentials request.
We opted for a modal for the UI of this interaction because a modal is more portable, and avoids customization of the same UI in multiple locations it will be used in (e.g. plugin cards, plugin list table, theme list table, theme cards, core upgrade screen).
The FS creds form is typically plopped on its own screen, and has as much room to breathe as any other Settings page, which it shares likeness in both markup and layout. Mel's mockup tightens the request filesystem credentials UI in a constrained space without complicating anything. Let's implement.
Attachments (12)
Change History (41)
#2
in reply to:
↑ 1
@
10 years ago
- Focuses accessibility added
Replying to ericlewis:
We should take this opportunity to refactor the filesystem request form markup. Its currently in a table layout, similar to a Settings page. Started breaking things out to that affect.
Totally agree, thanks very much :) adding accessibility focus since there's room for a11y improvements.
This ticket was mentioned in Slack in #core by drew. View the logs.
10 years ago
This ticket was mentioned in Slack in #design by drew. View the logs.
10 years ago
This ticket was mentioned in Slack in #core by jorbin. View the logs.
10 years ago
#7
@
10 years ago
Not 100% sure this shouldn't be it's own ticket, but right now there's no way to cancel out of the modal. There's no button and ESC didn't work.
See the image I attached (above) for how it look.s
This ticket was mentioned in Slack in #core by jorbin. View the logs.
10 years ago
#9
@
10 years ago
@ipstenu - I don't think that needs to be it's own ticket. I think it's part of us improving the UI and UX which is what this ticket aims for.
#10
@
10 years ago
- Add a "Cancel" button which closes the modal on click.
- Add bottom margin to form elements to match
<p>
tag margins. - Add margin in between connection types.
See previous screenshots for the current state of styles.
I am touching age-old markup and CSS and would love someone familiar with our current CSS best-practices to either provide feedback or take over the implementation here.
#12
follow-up:
↓ 15
@
10 years ago
- Keywords has-patch ui-feedback removed
Quick accessibility note - the cancel button should also handle focus assignment, and set focus back to the referring control.
This ticket was mentioned in Slack in #core by eric. View the logs.
10 years ago
#15
in reply to:
↑ 12
@
10 years ago
Replying to joedolson:
Quick accessibility note - the cancel button should also handle focus assignment, and set focus back to the referring control.
The "update link" gets removed from the DOM as soon as the modal open so we should chose a different element where to set focus back, maybe the "Select" checkbox? Please let me know your thoughts. See screenshot.
Not sure the "Updating" message should kick in at this point, same for the "Install" button in the plugin cards that changes in "Installing" even before you confirm or cancel your credentials.
#16
@
10 years ago
I'm not hugely bothered by the 'Updating' message - it's not totally inaccurate; the "Updating" process has begun, even if no actual files are being written yet. But I can see an argument for an intermediate notice at this stage of 'Requesting authentication' or something like that.
So, thinking about my own process, if I cancel from updating a plug-in in a situation like this, it's usually because I'm intending to switch contexts to find my FTP info for the site. If this is common, it probably doesn't matter too much exactly where the focus goes, as long as it's proximate to the initiation. The checkbox for the plug-in would probably work.
#17
@
10 years ago
- Focuses javascript added
- Keywords dev-feedback added
Thanks @joedolson :) Updated patch is a first accessibility treatment pass:
- add role=dialog and aria-labelledby on the modal container
- add aria-describedby to the relevant form field, targeting the existing descriptions
- I'd suggest to change "e.g." in "example:" for better readability
- JavaScript: move focus to the modal first input field when the modal opens
- JavaScript: constrain keyboard navigation inside the modal
- JavaScript: make the "close modal" function reusable
- JavaScript: close the modal when pressing Escape or clicking the overlay
- JavaScript: set focus back to the previously active element (if any) or a reasonable element ("Select" checkbox)
Would recommend a JavaScript review from some lead dev, when they have a chance :)
#18
@
10 years ago
Outstanding issues:
- check headings hierarchy
bit tricky because the same output is used for the modal (where the first heading should be H1) and "in page" when JavaScript is off.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
10 years ago
#20
@
10 years ago
Thanks for the help @afercia!
In attachment:31608.3.diff, riffing off attachment:31608.3.patch:
- Fix a bug that left/right keypress events triggered closing the modal.
- Move object property definitions to the top of the file, group methods.
- Simplify/combine logic for storing the jQuery element to return focus to after closing the request for credentials modal in
wp.updates.$elToReturnFocusToFromCredentialsModal
- Not head-over-heels for passing a jQuery event object into
wp.updates.requestFilesystemCredentials( e )
, but I'm on the fence about separating the logic requiring the event into its own method.
#21
@
10 years ago
Thanks very much @ericlewis!
Noticed a small thing, the modal should close also when clicking on the overlay, see:
$( '#request-filesystem-credentials-dialog [data-js-action="close"], #request-filesystem-credentials-dialog' ).on( 'click', function() { ...
#22
follow-up:
↓ 23
@
10 years ago
Gotcha @afercia. I missed the utility intended by that selector, and saw that it closed the modal when using the keyboard to toggle Connection Types. Let's use .notification-dialog-background
to target.
In attachment:31608.5.diff, bring back @afercia's functionality to close the modal when clicking on the backdrop.
#23
in reply to:
↑ 22
@
10 years ago
Replying to ericlewis:
and saw that it closed the modal when using the keyboard to toggle Connection Types
And I missed that :) Thanks very much @ericlewis
#24
follow-up:
↓ 27
@
10 years ago
This is getting pretty close. A couple of changes that I made:
- On small windows (those with a height less than 480px which is the size of the window ), the modal is now fullscreen to be more usable.
- Canceling now restores the original HTML instead of having "updating..." stay there
More work remains on making sure that canceling doesn't block you from trying again. Some flows that need to be refined and could use some more testing include:
- Click Update, cancel, click update again
- Click Update, enter bad credentials, cancel, click update again
- Click Update, cancel, click update on a different plugin
I think the error state modal (what comes back when a user has entered the wrong credentials) needs an a11y review.
#26
@
10 years ago
- Keywords needs-testing added; dev-feedback removed
@jorbin @ericlewis, what's left here? We need to wrap this up.
#27
in reply to:
↑ 24
@
10 years ago
In attachment:31608.7.diff, follow-up on jorbin's notes:
- Click Update, cancel, click update again
Works.
- Click Update, enter bad credentials, cancel, click update again
Works.
- Click Update, cancel, click update on a different plugin
Works.
attachment:31608.diff is a work in progress.
We should take this opportunity to refactor the filesystem request form markup. Its currently in a table layout, similar to a Settings page. Started breaking things out to that affect.
Keep in mind, this markup will support both the creds request UI in both the modal context and the single page context, we should style accordingly.