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

Prevent doodle from closing on outside click #2047

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

Plastikmensch
Copy link

Fixes #2024

Adds a new property to the doodle dispatch, which when set to true, replaces the onClose handler with a no-op, preventing the modal from closing when clicking outside.

I'm not sure this is the best way, but I thought I would offer this solution.

Adds a new property to the dispatch, which when set to true, replaces the onClose handler with a no-op, preventing the modal from closing.

Signed-off-by: Plastikmensch <[email protected]>
@ClearlyClaire
Copy link

Hm, I wonder if adding a confirmation modal like for the media metadata edit modal be better? Though it should only apply to clicking out, not clicking “Done” or “Cancel”

@Plastikmensch
Copy link
Author

Hm, I wonder if adding a confirmation modal like for the media metadata edit modal be better? Though it should only apply to clicking out, not clicking “Done” or “Cancel”

Yeah, I also thought about that, but I imagine that getting annoying really quickly, especially when accidentally clicking out multiple times. I'm not sure how common clicking out is though.
Pressing escape is also suppressed silently, so I thought doing the same with clicking would be consistent.
I think if a confirmation modal is added, it should also show when pressing escape.

@ClearlyClaire
Copy link

Another thing I'm seeing is that the modal can be pretty broken on very narrow screens, and in this case, the “Done” and “Cancel” buttons are not guaranteed to appear, while there's at least some margin that currently allows closing the modal.

That being said, I'm not sure any actual user would use a screen that narrow, and if it were to happen, I guess it would be on mobile, where the “Back” button should be properly handled.

@ClearlyClaire ClearlyClaire merged commit 910d2d9 into glitch-soc:main Jan 4, 2023
@Plastikmensch
Copy link
Author

Another thing I'm seeing is that the modal can be pretty broken on very narrow screens, and in this case, the “Done” and “Cancel” buttons are not guaranteed to appear, while there's at least some margin that currently allows closing the modal.

That being said, I'm not sure any actual user would use a screen that narrow, and if it were to happen, I guess it would be on mobile, where the “Back” button should be properly handled.

That was one of my concerns with this. Sadly I couldn't test this on mobile. But yes, the "Back" button should still close it.

I remember seeing an open issue to make the doodle more mobile friendly, but that requires considerable rework of the modal and I'm not sure even if that's done, the doodle feature would work well on mobile devices or narrow screens, due to canvas size.

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.

It's too easy to lose a sketch in the sketch pad
2 participants