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

Improve UI/UX of file attachments in new Composer UI #7799

Closed
4 of 6 tasks
NateWr opened this issue Mar 28, 2022 · 5 comments
Closed
4 of 6 tasks

Improve UI/UX of file attachments in new Composer UI #7799

NateWr opened this issue Mar 28, 2022 · 5 comments
Assignees
Labels
Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days.

Comments

@NateWr
Copy link
Contributor

NateWr commented Mar 28, 2022

Describe the problem you would like to solve
The file attachments UI components work fine in the new Composer component. However, they have not been styled appropriately. In some cases they are missing styles completely.

Describe the solution you'd like
The following needs to be done:

  • Add styles when selecting attachment source and selecting attachments
  • Reset focus properly if the "<-- back" button is selected
  • There's a small gap in time before the focus is moved into the selected file attacher. During this time the focus can "escape" the modal. Needs to be trapped for good.
  • If only one file attacher is detected, skip the selection UI.
  • Prevent adding the same attachment twice
  • FileAttacherFileStage uses this.items but should use this.files to be like all other FileAttachers

Who is asking for this feature?
Finishing work on #5717.

@NateWr NateWr added the Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days. label Mar 28, 2022
@NateWr NateWr added this to the 3.4 milestone Mar 28, 2022
@NateWr NateWr self-assigned this Mar 28, 2022
NateWr added a commit to NateWr/ui-library that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/ui-library that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/ui-library that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/ui-library that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/pkp-lib that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/pkp-lib that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/ojs that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/omp that referenced this issue Mar 31, 2022
NateWr added a commit to NateWr/ops that referenced this issue Mar 31, 2022
@NateWr
Copy link
Contributor Author

NateWr commented Mar 31, 2022

I decided to leave the following two features for later. They are nice to have but not urgent:

  • If only one file attacher is detected, skip the selection UI.
  • Prevent adding the same attachment twice

Otherwise, the following PRs apply styles to the file attacher, introduce a new ActionPanel component, and fix the various accessibility focus issues. They also apply the ActionPanel component to the admin page.

admin

PRs:
pkp/ui-library#193
#7811
pkp/ojs#3354
pkp/omp#1094
pkp/ops#258

@NateWr
Copy link
Contributor Author

NateWr commented Mar 31, 2022

@ewhanson would you be able to do a code review on this? I'm particularly interested in having you check the keyboard navigation functionality. This fixes some focus bugs related to accessibility, and those tend to be where there are differences between browsers, and since you're on Mac we're likely to get some different behaviour.

To test in the UI Library:

  • ActionPanel, FileAttacher, and Composer (attach files)
  • In the composer, try to use just the keyboard to open the file attachment modal, select a file attachment source, and attach files. When a new modal opens, the focus should go into that modal (tab once to see it move onto an element).
  • When navigating "back" in the file attacher, the focus should put you back where it was before you opened the file attacher.
  • When attaching a file, or closing the modals with esc, the focus should move back to the "Attach Files" button in the Composer.
  • When tab or shift+tab and the next element will take you "outside" of the modal, it should keep you in the modal. (For example, hitting tab on the last element of the modal should move focus to the top of the modal, and vice-versa.)

To test in OJS:

  • Record any editorial decision where you notify an author and try to attach files.

@ewhanson
Copy link
Collaborator

ewhanson commented Apr 1, 2022

Hey @NateWr, I'll have a look. Thanks!

@ewhanson
Copy link
Collaborator

ewhanson commented Apr 5, 2022

Hey @NateWr, I've reviewed the code and done some keyboard navigation testing. The code all looked good, but I ran into a few issues with Firefox on Mac and (perhaps not surprisingly) a number of issues with Safari on Mac. Anything that came up in Firefox was also applicable in Safari.

This also wasn't explicitly in this PR, but the readme file for the FileAttacher component still has placeholder text in it (This is some text.).

Firefox

Composer (UI Library)

  • When in attach review files modal, can't keyboard navigate to download button.
  • When in attach submission files modal, after opening the "Other files" dropdown, navigation doesn't loop back to the top of the dropdown and instead moves on to the next item outside of the dropdown. I thought this was probably the expected behaviour, but wanted to flag it just in case.

In OJS

  • When submitting a "Send for review" decision, the "sent for review" modal that follows is not tab navigable.
  • On the Admin page, the tab navigation skips the "Site Management," "System Information," and "Jobs" sections. Only the "red buttons" are tab navigable.

Safari

ActionPanel (UI Library)

  • Can't tab at all into ActionPanel.
  • Even if a button in ActionPanel is selected with the mouse, when the modal is dismissed, focus is not given to the original calling button.
  • With the modal open (Delete incomplete submissions), only the closing X can receive focus.

Composer (UI Library)

  • Only the "find template," "subject," and "body" text areas can receive focus. Attach files button can't be navigated to via keyboard at all.
  • Same issue as above with open modal. Only the closing X can receive focus.
  • If attach files modal is open, pressing esc will set focus to the "attach files" button, but once the user navigates away from the button, they cannot navigate back to it.

In OJS

  • On the Admin page, none of the ActionPanel items are tab navigable.

NateWr added a commit to NateWr/pkp-lib that referenced this issue Apr 5, 2022
NateWr added a commit to NateWr/pkp-lib that referenced this issue Apr 5, 2022
NateWr added a commit to NateWr/ojs that referenced this issue Apr 5, 2022
NateWr added a commit to NateWr/omp that referenced this issue Apr 5, 2022
NateWr added a commit to NateWr/ops that referenced this issue Apr 5, 2022
NateWr added a commit that referenced this issue Apr 5, 2022
#7799 Fix fileStages param in submission file API and use ActionPanel on admin page
NateWr added a commit to pkp/ui-library that referenced this issue Apr 5, 2022
pkp/pkp-lib#7799 Add ActionPanel and improve UX of FileAttacher components
NateWr added a commit to pkp/omp that referenced this issue Apr 5, 2022
NateWr added a commit to pkp/ops that referenced this issue Apr 5, 2022
NateWr added a commit to pkp/ojs that referenced this issue Apr 5, 2022
@NateWr
Copy link
Contributor Author

NateWr commented Apr 5, 2022

After a short call with Erik, the issues were all related to a system default in Mac that alters keyboard navigation. 😅

I've merged all the PRs to main.

@NateWr NateWr closed this as completed Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement:1:Minor A new feature or improvement that can be implemented in less than 3 days.
Projects
None yet
Development

No branches or pull requests

3 participants