Skip to content

Conversation

@jrochkind
Copy link
Member

@jrochkind jrochkind commented Nov 10, 2025

Originally modal.js could handle form submissions in a modal as well as links.

But this behavior wasn't actually used by Blacklight, or tested -- it was available for Blacklight apps to use though. It made sense to support since it was a variation on the hyperlink behavior, keep it DRY.

But it was removed in some prior commits refactoring, perhaps be61504, if not before -- it was quite possibly already broken at that point.

This PR adds it back in, in case it is desired by community!

i got to needing this for a local app, but if it is not desired to be part of BL, it's okay with me, I can easily patch it back in locally only in my app.

It does not at present have tests. it's kind of a pain to have tests, since there is no behavior in default BL that exersizes it, we'd have to figure out how to add exersizing behavior to test app somehow, which engine_cart makes difficult. If there is interest in this feature, and people believe tests are necessary, perhaps we can join heads on how to do it.

Closes #2331

@jrochkind jrochkind marked this pull request as draft November 10, 2025 22:12
@ebenenglish
Copy link
Contributor

@jrochkind, does this fix #2331?

@jrochkind
Copy link
Member Author

i didn't notice that issue @ebenenglish , but it does! i meant to fill out total description here before I submitted, will convert to draft until it's totally done.

@jrochkind jrochkind marked this pull request as ready for review November 10, 2025 22:14
@jrochkind jrochkind marked this pull request as draft November 10, 2025 22:14
@jrochkind jrochkind marked this pull request as ready for review November 10, 2025 22:18
@jcoyne
Copy link
Member

jcoyne commented Nov 11, 2025

Do we need this in Blacklight? Blacklight doesn't have forms in the modal, right? Can this go in your own app?

We have forms in our modal, but we wouldn't want the behavior you have defined here. Our form submission closes the modal and shows feedback in a "toast".

@jrochkind
Copy link
Member Author

jrochkind commented Nov 11, 2025

@jcoyne As I wrote in description already:

This PR adds it back in, in case it is desired by community!

i got to needing this for a local app, but if it is not desired to be part of BL, it's okay with me, I can easily patch it back in locally only in my app.

and:

But this behavior wasn't actually used by Blacklight,

One possible additional question:

We have forms in our modal, but we wouldn't want the behavior you have defined here. Our form submission closes the modal and shows feedback in a "toast".

The behavior here is opt-in with a data attribute on your form (just as it already was for links, not every link in a modal) if this behavior were merged, it would not affect your forms in modals.

jrochkind added a commit to sciencehistory/scihist_digicoll that referenced this pull request Nov 11, 2025
jrochkind added a commit to sciencehistory/scihist_digicoll that referenced this pull request Nov 11, 2025
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.

data-blacklight-modal=preserve doesn't work for forms

4 participants