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 collapse behavior #10467

Merged
merged 2 commits into from
Aug 14, 2024
Merged

Improve collapse behavior #10467

merged 2 commits into from
Aug 14, 2024

Conversation

nwalters512
Copy link
Contributor

This is being done ahead of Bootstrap 5. In v5, they changed how data-toggle="collapse" is handled. Now, they handle click events for that in the capturing phase instead of the bubbling phase (see twbs/bootstrap#36063). This means that we no longer have the option of using stopPropagation() to ensure that clicks on nested buttons don't toggle the open/closed state. I opened twbs/bootstrap#40574 to change Bootstrap to handle these events in the bubbling phase, but that's unlikely to be reviewed or released soon.

Now, collapse toggling is controlled only by an explicit button. The main user-facing change is that clicking on question submission/feedback panel headers will no longer toggle the open/closed state of the panel. IMO this is strictly (if indeed slightly) worse, but 🤷

I made some accessibility improvements as I went:

  • I added some missing aria-expanded/aria-controls attributes
  • I changed pl-external-grader-results to use a button as a toggle instead of a div. This ensures that it can be navigated and operated with a keyboard.

Copy link
Contributor

github-actions bot commented Aug 14, 2024

All images

Image Platform Old Size New Size Change
prairielearn/executor:6e4881fb5d8e3134bfa8c4a2ef2cf6b533f64c0d null 1185.44 MB 1185.27 MB -0.01%
prairielearn/prairielearn:6e4881fb5d8e3134bfa8c4a2ef2cf6b533f64c0d linux/amd64 1185.43 MB 1185.27 MB -0.01%

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

Attention: Patch coverage is 86.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 71.62%. Comparing base (10db4a2) to head (6e4881f).
Report is 1 commits behind head on master.

Files Patch % Lines
...ielearn/src/components/CourseRequestsTable.html.ts 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10467      +/-   ##
==========================================
+ Coverage   71.61%   71.62%   +0.01%     
==========================================
  Files         526      526              
  Lines       82763    82765       +2     
  Branches     6935     6939       +4     
==========================================
+ Hits        59268    59284      +16     
+ Misses      23033    23018      -15     
- Partials      462      463       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nwalters512 nwalters512 mentioned this pull request Aug 14, 2024
12 tasks
@nwalters512 nwalters512 added this pull request to the merge queue Aug 14, 2024
Merged via the queue into master with commit 291baa1 Aug 14, 2024
9 checks passed
@nwalters512 nwalters512 deleted the improve-collapse-behavior branch August 14, 2024 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants