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

Gh 4515 and 4516 batch deactivate #4538

Merged
merged 2 commits into from
Oct 7, 2020
Merged

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Oct 7, 2020

Restoring HTML form field name.

a09812c

In 241cbed we introduced changes to
address accessibility issues reported in #3966. However, the introduced
change broke the HTML form field names and the assumed parameters for
the batch processing.

This commit reverts that change, but it does not address the
accessibility concern raised in #3966. That is for another commit, as I
don't know if that other commit is the correct path forward.

Closes #4516
Closes #4515

Adding fieldset to address like named form inputs

b152637

In consultation with @Dananji and following the guidance of W3.org,
this commit wraps the forms (or controls) that render the
add_buttom partial. I don't believe that I have access to the
accessibility testing software, so this is a bit of a stab in the dark.

Note, in some cases I render the fieldset as a direct child of the form,
in other cases, I render the fieldset as a container for the table used
to present the elements that render the add button partial.

This relates to work done in #4160 to address #3966.

@samvera/hyrax-code-reviewers

In 241cbed we introduced changes to
address accessibility issues reported in #3966.  However, the introduced
change broke the HTML form field names and the assumed parameters for
the [batch processing][1].

This commit reverts that change, but it does not address the
accessibility concern raised in #3966.  That is for another commit, as I
don't know if that other commit is the correct path forward.

Closes #4516
Closes #4515

[1]:https://github.com/samvera/hyrax/blob/3c7258c95c1fe6f0fe1537078f3f5f458f211989/app/controllers/concerns/hyrax/collections/accepts_batches.rb#L20-L28
In consultation with @Dananji and following the [guidance of W3.org][1],
this commit wraps the forms (or controls) that render the
[add_buttom partial][2].  I don't believe that I have access to the
accessibility testing software, so this is a bit of a stab in the dark.

Note, in some cases I render the fieldset as a direct child of the form,
in other cases, I render the fieldset as a container for the table used
to present the elements that render the add button partial.

This relates to work done in #4160 to address #3966.

[1]:https://www.w3.org/WAI/tutorials/forms/grouping/
[2]:https://github.com/samvera/hyrax/blob/d5aa2f9ca802fe670687bb4f7c14ad242c1bacf6/app/views/hyrax/batch_select/_add_button.html.erb#L2
@jeremyf jeremyf requested a review from Dananji October 7, 2020 15:26
Copy link
Contributor

@Dananji Dananji left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@jeremyf jeremyf merged commit 8384679 into master Oct 7, 2020
@jeremyf jeremyf deleted the gh-4515-and-4516-batch-deactivate branch October 7, 2020 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants