Skip to content

Conversation

@jrochkind
Copy link
Contributor

@jrochkind jrochkind commented Nov 11, 2025

We had to locally add "keep form submits in modal behavior", that used to be in Blacklight but got removed some time ago. We patched it back into Blacklight instead of just adding our own independent thing, and contributed it upstream, but Blacklight may not want it upstream. There is some risk of future Blacklight breaking our patch, if that ever happens we should probably just extract it completely.

The OH request workflow still works not in a modal too, if you access a request URL directly, or say click on requet button with "open in new tab". The Blacklight modal stuff is meant to support that, and I like it as a failsafe.

It does mean we have a lot more conditional logic in our already hacky conditional-heavy oral history requests controller. Tried to keep it clean with a helper method, but it is a bit confusing. We need slightly different status messages in the modal case for instance, because we need a link to Your Requests in some cases, when in the non-modal case we are redirecting there!

There were existing capybara specs -- which actually just kept working (and not false positive!) -- cause they were checking for text on screen after submitting request, and whether that text is in a modal or a flash notice at top of screen, either way test passed! I did add a bit to tests to actually ensure modal is on screen though!

  • Patch blacklight form-in-modal behavior back in
  • OH request use new form in modal behavior
  • clean up layout for OH request in modal

.

  • Demo and get stakeholder approval before merging

@jrochkind jrochkind marked this pull request as ready for review November 18, 2025 15:49
@jrochkind
Copy link
Contributor Author

jrochkind commented Nov 18, 2025

@eddierubeiz approve at your convenience!

@jrochkind jrochkind merged commit f35c819 into master Nov 20, 2025
1 check passed
@jrochkind jrochkind deleted the modal_form branch November 20, 2025 12:45
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.

3 participants