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

Address transfer submission process based on user feedback #695

Merged
merged 1 commit into from
Apr 29, 2021

Conversation

matt-bernhardt
Copy link
Member

@matt-bernhardt matt-bernhardt commented Apr 21, 2021

Ticket: https://mitlibraries.atlassian.net/browse/ETD-283

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • VoiceOver has been run in accordance to our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

@mitlib mitlib temporarily deployed to thesis-submit-pr-695 April 21, 2021 18:46 Inactive
@coveralls
Copy link

coveralls commented Apr 21, 2021

Coverage Status

Coverage remained the same at 94.513% when pulling 6e8e336 on etd-256-transfer-punchlist into 37fe064 on main.

@JPrevost JPrevost had a problem deploying to thesis-submit-pr-695 April 26, 2021 18:47 Failure
@JPrevost JPrevost had a problem deploying to thesis-submit-pr-695 April 26, 2021 19:36 Failure
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-695 April 26, 2021 20:13 Inactive
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-695 April 26, 2021 21:51 Inactive
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-695 April 26, 2021 21:55 Inactive
@matt-bernhardt matt-bernhardt force-pushed the etd-256-transfer-punchlist branch from f480724 to df59af1 Compare April 27, 2021 15:27
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-695 April 27, 2021 15:28 Inactive
@matt-bernhardt matt-bernhardt requested review from JPrevost and jazairi and removed request for JPrevost April 27, 2021 15:57
@JPrevost JPrevost self-assigned this Apr 27, 2021
Copy link
Member

@JPrevost JPrevost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirmed voice over does read the changing part but then just abruptly moved to complete. That's probably okay, but wanted to note that it was pretty janky still but I think it's likely very good for slower uploads and just acts weird for a bunch of small files.

I might have missed the new visual indicators if I didn't know to scroll to see them as they were just off the bottom of my screen. It might be good to change focus? Not sure. Thoughts?

@matt-bernhardt matt-bernhardt force-pushed the etd-256-transfer-punchlist branch from df59af1 to 2be2305 Compare April 28, 2021 21:22
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-695 April 28, 2021 21:22 Inactive
** Why are these changes being introduced:

* The default rendering of in-progress uploads to S3 (provided by
  boilerplate code around direct upload) is inaccessible to screen
  readers, and is visually indistinct.
* Some of the wording on the new transfer form needs to be adjusted
  based on the results of user testing, and the subsequent changes.
* A link to DSpace@MIT needs to be added to the Transfer confirmation
  page.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/ETD-256

** How does this address that need:

This updates the JS and CSS used to render the direct upload progress
indicators. Specific changes include:

* Moving the upload panel to be visually just above the form submit
  button, which reduces the chance that the user will miss them (being
  off-screen in the middle of the form). Additionally, the upload
  process will set the URL hash to this newly-created panel in case
  it should appear offscreen.
* Adds an ARIA live region at the top of the panel that provides a
  summary of the upload process which can be read by screen readers.
  The panel updates as each file is uploaded.
* Applies our branding package to the individual file panels, making
  them more visually prominent.

Beyond the file upload feedback, we also make the following changes:

* Updates the form display to take the full width of the content column.
* Re-writes the text that appears in the well above the form based on
  UX-provided copy.

** Document any side effects to this change:

* We had already made changes to the boilerplate JS and CSS, but this PR
  will intervene more heavily. I compared our current file (which we
  added to the repo some time ago) with the version in the current
  documentation, and it does not seem to have been updated in the years
  since - so there appears to be little risk in us needing to implement
  subsequent changes.
@matt-bernhardt matt-bernhardt force-pushed the etd-256-transfer-punchlist branch from 2be2305 to 6e8e336 Compare April 28, 2021 22:02
@matt-bernhardt matt-bernhardt temporarily deployed to thesis-submit-pr-695 April 28, 2021 22:02 Inactive
@matt-bernhardt
Copy link
Member Author

Ok, I've put up an updated commit that incorporates the feedback from you and Rich, and the conversations that I've had with you and Stephanie. This PR now includes both the changes to the upload progress, as well as the wording changes from Stephanie.

@matt-bernhardt matt-bernhardt requested a review from JPrevost April 29, 2021 14:43
@matt-bernhardt matt-bernhardt merged commit c36a9ac into main Apr 29, 2021
@matt-bernhardt matt-bernhardt deleted the etd-256-transfer-punchlist branch April 29, 2021 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants