Problem/Motivation

Currently, modals created by Project Browser don't have their HTML removed from the DOM after they're closed. If the same "View Commands" button is clicked repeatedly, it creates new HTML that isn't removed until the page is refreshed or navigated away from.

Steps to reproduce

Use either major version of Project Browser - both 1.0.x-dev and 2.0.x-dev branches have this behavior.

CommentFileSizeAuthor
#14 Screencast 2024-08-15 13-22-21.mp45.24 MBsnehal-chibde
#7 close-modal.mp418.7 MBsnehal-chibde
modals-not-removed.mp42.59 MBkwiseman

Issue fork project_browser-3458908

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kwiseman created an issue. See original summary.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Issue tags: +stable blocker

omkar-pd made their first commit to this issue’s fork.

omkar-pd opened merge request !562

omkar-pd’s picture

omkar-pd commented
Status: Active » Needs review
atul_ghate’s picture

atul_ghate commented
Assigned: Unassigned » atul_ghate

I will review this issue.

snehal-chibde’s picture

snehal-chibde commented
Assigned: atul_ghate » snehal-chibde
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new close-modal.mp418.7 MB

hello @omkar-pd, I have tested the code in version 2.0.x-dev and now the modals are getting removed from DOM on click of close button. Added a video recording for reference.

atul_ghate’s picture

atul_ghate commented

Hi
I have reproduced the issue successfully...
I have tested MR 562 on the Drupal 10.3 version,
The MR is applied cleanly....

Test Result:
Modals created by Project Browser do have their HTML removed from the DOM after they're closed.

RTBC+1

chrisfromredfin changed the visibility of the branch 3458908-remove-modals-after to hidden.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Needs review

I found that this would not remove the HTML if the dialog was closed using ESC. Using the Drupal.dialog() api, you can specify a handler to invoke when the modal is closed. I moved the code into there and it seems to be working.

Could someone re-test this with the latest, review my code, ensure it's working, and set it back to RTBC?

omkar-pd’s picture

omkar-pd commented
Assigned: snehal-chibde » Unassigned
Status: Needs review » Needs work

Tests failed.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs work » Needs review

Needs manual tests, but automated tests were failing for flakiness only (retests passed).

omkar-pd’s picture

omkar-pd commented
Status: Needs review » Reviewed & tested by the community

Tested this locally. Removes Modals dialog on Esc and on close.

snehal-chibde’s picture

snehal-chibde commented
StatusFileSize
new Screencast 2024-08-15 13-22-21.mp45.24 MB

hello @chrisfromredfin, I have tested the code in version 2.0.x-dev and now the modals are getting removed from DOM on click of ESC button and Close Icon both. Added a video recording for reference.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Fixed

Thanks all, a good little cleanup!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.