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

Streamline bulk actions use of dynamic links to use HTML instead of JavaScript #11770

Open
lb- opened this issue Mar 17, 2024 · 3 comments
Open

Comments

@lb-
Copy link
Member

lb- commented Mar 17, 2024

Is your proposal related to a problem?

We currently have a large amount of JavaScript in the bulk actions code that does the following;

  1. Track the selected element's and their matching model instance ID.
  2. Intercept the clicks of links (for the bulk actions) and disable their default behaviour.
  3. When the link is clicked, build up the collected IDs into a URL param structure.
  4. Then, change the current URL to that new built up URL.

Some of the code that does this.

function onClickActionButton(e) {
e.preventDefault();
const url = e.target.getAttribute('href');
const urlParams = new URLSearchParams(window.location.search);
if (checkedState.selectAllInListing) {
urlParams.append('id', 'all');
const parentElement = document.querySelector(BULK_ACTIONS_CHECKBOX_PARENT);
if (parentElement) {
const parentPageId = parentElement.dataset.bulkActionParentId;
urlParams.append('childOf', parentPageId);
}
} else {
checkedState.checkedObjects.forEach((objectId) => {
urlParams.append('id', objectId);
});
}
window.location.href = `${url}&${urlParams.toString()}`;
}

This JavaScript could very likely be completely removed by using a HTML only approach.

Describe the solution you'd like

  • Using the HTML form element and setting the checkbox input name & value attributes correctly negates the need for tracking the id with data-object-id.
  • Instead of the bulk action buttons being an a element, we could use button and then target the form with the form="..." attribute. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#form
  • The bulk action button elements can direct the action to a custom base URL by setting the formaction attribute. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#formaction
  • The next URL value can be declared with a hidden input that associates to the same form.
  • The dynamic 'Select all items in this listing' button can be changed to an input with a label and have the name 'all' and the value would be determined on clicking.
  • The dynamic "All ... in listing selected" label could move after the input checkbox and then we can easily support turning on / off this 'select' all behaviour plus make this label hide & show based on CSS only (no JS).
  • The childOf value could be a simple hidden input that gets created when the parent exists, the bulk actions response can conditionally handle this when all is selected. By the looks of things, this is current behaviour so no change needed to the wagtail/admin/views/bulk_action/base_bulk_action.py code.
  • We would also get the benefit of ctrl+click (cmd+click) for bulk actions working nicely (in browsers that support that, Chrome/Safari).

While this seems like a lot it will mean that we can pull out a lot of JavaScript code and replace it with simpler semantic HTML that has the following benefits;

  1. The bulk action buttons will now be actual button elements, not a elements.
  2. We will not be breaking the a link behaviour.
  3. Screen readers will better associate those buttons with the form that contains the checkboxes, making navigation around the page better.
  4. Adding new base Params to the bulk actions URLs will be as easy as adding a new hidden input in the template, no complex code overrides needed (likely it's not even possible to do this today).
  5. Makes way for easier Stimulus migration for the bulk actions code.

Potential approach

  1. Create a new template tag that will replace the shared include wagtail/admin/templates/wagtailadmin/bulk_actions/footer.html.
  2. This new template tag would replace the bulk_action_choices template tag and own that same responsibility of preparing the actions. It would need to accept a param for the form id to attach the inputs to.
  3. We would need to modify wagtail/admin/templates/wagtailadmin/shared/button.html to render an actual button element (conditionally) or maybe we make two; one for links and one for buttons. The button variation would need to render the url as the formaction attribute and then pull in a form_id value to render. OR we can just leverage the attrs for this approach.
  4. We would ensure this footer actions template renders the input for the next value and we can pull out the appending of the URL ?next=... in the template tag.

Describe alternatives you've considered

  • No change, we can move all the JS to the new Stimulus controller migration.
  • Maybe there is some reason we opted for NOT using the HTML approach in the past?

Additional context

  • A starting point has been created here for reference lb-@d9950d3
Screenshot 2024-03-17 at 8 23 42 pm

Working on this

  • This will not be a simple undertaking, while anyone can contribute to this we encourage you to review the proposal in full and ensure you fully understand how bulk actions work (across all model types) and how the form and input / button interaction works, additionally some solid understanding of Django template tags would be required.
  • View our contributing guidelines, add a comment to the issue once you’re ready to start.
@lb-
Copy link
Member Author

lb- commented Mar 17, 2024

@laymonage - would love your thoughts on this.

I feel this could be done in smaller steps with some other contributor's support but I wanted to check you are OK in principle with this, additionally flag anything I may have missed in this issue or with the original JS only approach.

@mqnifestkelvin
Copy link

Hi I am applying for the Google summer of code and I am interested in wagtail's project as well as the community, I really like what you do as it aligns with what I current am interested in and looking to improve at. Would this issue be a good first issue to start of as on as Google summer of code applicant?

@lb-
Copy link
Member Author

lb- commented Mar 23, 2024

@mqnifestkelvin this is not a good first issue, please read the Working on this section in the issue description. It explains what kind of contribution this would be.

Good luck with your GSoC application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants