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

Set aria-busy on the form element during a form submission #1110

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

afcapel
Copy link
Collaborator

@afcapel afcapel commented Dec 18, 2023

This is useful for styling the form while it is submitting.

Before this change, we were only setting aria-busy on frames while they were loading and on the html element while Turbo was processing a visit.

This is useful for styling the form while it is submitting.

Before this change, we were only setting aria-busy on frames while they
were loading and on the html element while Turbo was processing a visit.
@dhh
Copy link
Member

dhh commented Dec 18, 2023

Wonder if we actually need to set it on the form element itself or if setting it on the body is enough? With frames, do we set it just on the frame or on the root document body? I guess if frames set it on their own element, setting busy on the form makes sense too.

@afcapel
Copy link
Collaborator Author

afcapel commented Dec 18, 2023

Yes, frames set it on their element. I think it's better to set it on the form, just in case there are several forms in the page.

@afcapel afcapel merged commit df7f982 into main Dec 18, 2023
2 checks passed
@afcapel afcapel deleted the form-aria-busy branch December 18, 2023 16:37
@seanpdoyle
Copy link
Contributor

seanpdoyle commented Dec 18, 2023

According to the specification, elements with role="form do support the [aria-busy] attribute.

The aria-busy portion of the specification mentions that it

Indicates whether an element, and its subtree, are currently being updated.

I think there's some room to interpret what "being updated" means in the context of a <form> element.

With that being said, setting <body aria-busy="true"> might supersede anything that Turbo would do to elements deeper in the document, which has the potential to make this change moot from an assistive tech perspective.

@afcapel
Copy link
Collaborator Author

afcapel commented Dec 18, 2023

@seanpdoyle as far as I can tell we're not setting [aria-busy] on the body on form submissions, only on page navigations.

@dhh
Copy link
Member

dhh commented Dec 18, 2023

Given that we already have it scoped with frames, it seems best to follow that pattern with forms too. Especially, as @afcapel says, you could have multiple forms per page, and we want to be able to use aria-busy to style which one is submitting.

afcapel added a commit to pfeiffer/turbo that referenced this pull request Jan 29, 2024
* origin/main:
  Introduce `turbo:{before-,}morph-{element,attribute}` events
  Turbo 8.0.0-beta.4
  Introduce data-turbo-track="dynamic" (hotwired#1140)
  Ensure that the turbo-frame header is not sent when the turbo-frame has a target of _top (hotwired#1138)
  Turbo 8.0.0-beta.3
  Fix attribute name (hotwired#1134)
  Add InstantClick behavior (hotwired#1101)
  Revert hotwired#926. (hotwired#1126)
  Keep Trix dynamic styles in the head (hotwired#1133)
  Remove unused stylesheets when navigating (hotwired#1128)
  Upgrade idiomorph to 0.3.0 (hotwired#1122)
  Debounce page refreshes triggered via Turbo streams
  Update copyright year to 2024 (hotwired#1118)
  Turbo 8.0.0-beta.2
  Set aria-busy on the form element during a form submission (hotwired#1110)
  Dispatch `turbo:before-fetch-{request,response}` during preloading (hotwired#1034)
domchristie pushed a commit to domchristie/turbo that referenced this pull request Jul 20, 2024
…1110)

This is useful for styling the form while it is submitting.

Before this change, we were only setting aria-busy on frames while they
were loading and on the html element while Turbo was processing a visit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants