Skip to content

Maintain focus during removal of supervisor field#689

Merged
matt-bernhardt merged 1 commit into
mainfrom
etd-272-remove-focus
Apr 21, 2021
Merged

Maintain focus during removal of supervisor field#689
matt-bernhardt merged 1 commit into
mainfrom
etd-272-remove-focus

Conversation

@matt-bernhardt

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

Copy link
Copy Markdown
Member

This fixes the issue where focus is left undefined when a supervisor field is removed.

It also fixes a separate-but-related bug where the "remove this supervisor" link is not appropriately removed on the edit form in some conditions.

Note: I'm not sure how to use ANDI or Wave when changes like this are made - but I have tested this code under keyboard navigation and using visual cues under my locally-installed browsers (Firefox, Chrome, and Safari).

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave 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-689 April 9, 2021 20:24 Inactive
@coveralls

coveralls commented Apr 9, 2021

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 94.513% when pulling 5be15bf on etd-272-remove-focus into e0ee17c on main.

@matt-bernhardt matt-bernhardt changed the title Set focus to final supervisor field on removal Maintain focus during removal of supervisor field Apr 16, 2021
** Why are these changes being introduced:

* When the user removes a thesis supervisor field, the focus is left
  undefined. This has been flagged during an accessibility review, with
  a preference stated that focus be set explicitly.
* Separately, we have identified a bug in the current edit form where
  the "remove this supervisor" link is not being correctly hidden.
  These two issues share a need to accurately count how many fields are
  visible, because fields are not always removed - but hidden.

** Relevant ticket(s):

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

** How does this address that need:

* This defines a new function within student_thesis_form.js, to set
  focus on the first visible supervisor field. This function is called
  during the cocoon:after-remove hook.
* The separate bug in the edit form shares a common concern with this
  focus work, which is to identify how many fields are visible. Our
  previous work here just counted "a.remove" links, which turns out to
  be inadequate because an edit form does not remove existing fields -
  just hides them.
* Both routines now rely on a shared function that counts how many
  visible fields exist.

** Document any side effects to this change:

Our use of javascript on the student-facing thesis forms is now more
complex.
@JPrevost JPrevost self-assigned this Apr 21, 2021
@matt-bernhardt matt-bernhardt merged commit 7f2028f into main Apr 21, 2021
@matt-bernhardt matt-bernhardt deleted the etd-272-remove-focus branch April 21, 2021 15:19
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