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

Remember checkbox state following page navigation #764

Merged
merged 5 commits into from
Feb 14, 2025
Merged

Conversation

rw251
Copy link
Contributor

@rw251 rw251 commented Feb 6, 2025

Uses sessionStorage to persist the current state of checkboxes. As currently implemented, it will remember checkboxes until you open airlock in a new window. It uses the baseURI and value to uniquely identify each checkbox.

I also took the opportunity to improve the "select all" checkbox so that:

  • if you select all, and then deselect one, the select all checkbox unchecks
  • if you select all checkboxes individually then the select all checkbox checks
  • if some but not all are checked then the select all checkbox enters the "intermediate" state and shows the "partially selected" style

@rw251 rw251 force-pushed the rw/remember-checkboxes branch 3 times, most recently from 8f34329 to 6fa9226 Compare February 11, 2025 15:00
There appear to be two situations in which the back button interferes with the checkbox caching:
1. After a back button click the current state is not rendered until a page refresh
2. After a back button click, the event listener is sometimes not added resulting in any checkbox changes not persisting to sessionStorage

The two failing tests added here capture both flows.
@rw251 rw251 force-pushed the rw/remember-checkboxes branch from 6fa9226 to c37fc6d Compare February 14, 2025 11:41
@@ -133,8 +133,22 @@ function addCheckboxClickListener() {

// On first load of the page we need to wire up the event listener
// so that we can respond to checkbox changes.
document.addEventListener("DOMContentLoaded", addCheckboxClickListener);
if (document.readyState !== "loading") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this because of the htmx loads? So the document itself is already loaded but the checkbox listener and status have to be called again for htmx-refreshed content?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's actually just for the back button case. The event listener is put on an element far enough up the DOM so that any htmx changes within the file browser don't touch it. Once its enabled initially you therefore don't need to redo it. However, when the back button is clicked you get a whole page refresh so event listeners are disregarded - but you also don't get a DOMContentLoaded event (or at least it has already triggered before this code runs). I think this is because the page is cached by the browser in some way, but I'm not 100%.

@rw251 rw251 merged commit 36db812 into main Feb 14, 2025
10 checks passed
@rw251 rw251 deleted the rw/remember-checkboxes branch February 14, 2025 15:16
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.

2 participants