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

Fix metadata dialog backdrop click to exit #6421

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

viown
Copy link
Member

@viown viown commented Jan 2, 2025

Changes

  • Fixes not being able to click the backdrop of the metadata dialog to exit
  • Fixes dragging mouse from inner dialog to outside causing it to close (Firefox)

Issues
Fixes #5723

@viown viown requested a review from a team as a code owner January 2, 2025 10:44
@viown viown added bug Something isn't working stable backport Backport into the next stable release labels Jan 2, 2025
@jellyfin-bot
Copy link
Collaborator

jellyfin-bot commented Jan 2, 2025

Cloudflare Pages deployment

Latest commit a553ef5
Status ✅ Deployed!
Preview URL https://36d0a248.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs

@viown viown force-pushed the fix-metadata-outer-click branch from cac64ac to a553ef5 Compare January 2, 2025 10:45
Copy link

sonarqubecloud bot commented Jan 2, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@nielsvanvelzen
Copy link
Member

When a dialog contains forms it should require an explicit close button to be pressed or an additional dialog that shows to confirm with the user that any changes made to the form will not be saved. It is extremely easy to close a dialog using a click on the scrim by accident.

@viown
Copy link
Member Author

viown commented Jan 2, 2025

I'm not sure how I feel about a confirmation dialog when trying to click out through the backdrop. Ideally, it should only show up when there are unsaved changes in the form but that's beyond this bugfix PR (clicking in dialog and releasing the mouse outside it causing it to close).

I also think that disabling the click for some dialogs while others not is inconsistent and annoying to the user. I'm not sure how common it is to accidentally click outside, but with this PR it should be a bit harder to do so.

@thornbill thornbill added this to the v10.10.4 milestone Jan 3, 2025
@thornbill
Copy link
Member

I think we massively overuse modals, and I hate it, but this makes the modal function the same as all the others while fixing the original bug.

@thornbill thornbill merged commit e1deddc into jellyfin:release-10.10.z Jan 3, 2025
15 checks passed
@viown viown deleted the fix-metadata-outer-click branch January 13, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stable backport Backport into the next stable release
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants