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

Modal Dialogs #839

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Modal Dialogs #839

wants to merge 5 commits into from

Conversation

plasticartsshow
Copy link

This adds modal dialogs as described in issue 686, closely following the way popups are implemented.

The modal dialog:
  • Is shown with a function and stores the bare minimum state info in the ui context,
  • Enforces a user-interface mode by intercepting clicks/interactions, preventing the user from interacting with the covered ui. (See Modal & Nonmodal Dialogs: When (& When Not) to Use Them),
  • Enforces a logical mode: Whichever id first claims the modal "owns" it, and another id cannot be used to show a modal without relinquishing control established by the first (Relinquishing control doesn't require id, just an empty function call). This prevents a modal from inadvertently being dismissed by another modal, which could confuse application flows,
  • Lets the user customize the modal interceptor background color/transparency,
  • Makes the user is wholly responsible for the ui shown in the modal, and
  • Supports the modal being optionally closed using keyboard keys (as shown in the example).
Screen cap showing modal interception of ui senses

egui_modal

Possible Problems / Needs improvement
  • Implementation: Content in the modal is shown in an Area. Using an Area inside the modal ui requires that the inner Area be set to Order::Foreground so it shows up on top of the modal interceptor. This may not be intuitive.
  • Usability: It would have been nice wanted to be able to give the user the option to click away from the modal and close it, but that required making set-in-stone decisions about the ui and complicating the function call. The implementer ought still be able to provide this functionality, however.
  • Aesthetic: This doesn't make any decisions about aesthetic, so by default, it's not an exact match for the general egui look. It was thought that a broader approach could serve a lot of different purposes, such as showing error messages or character overlays like in RPG-style games, etc.
In addition to the above, this PR:
  • Adds a line to the relevant CHANGELOG.md under "Unreleased".
  • Has some example code (doc-tests)
  • Has a demo for it inegui_demo_lib.
  • Assuages the savage sh/check.sh.
  • When you have addressed a PR comment, mark it as resolved. (Unsure if some requirement was left unsatisfied, so leaving this for now)

Closes #686.

Give a shout if anything is wrong here.

@emilk
Copy link
Owner

emilk commented Oct 25, 2021

Thanks for starting this!

The problem with a show_custom_modal is that it is difficult for the user to choose options for the modal area.

I would consider an alternative, where Memory keeps track of a modal stack of LayerId. If it is empty, no modal window is open. If non-empty, the top layer is the only one that gets input (gate-keeped by Context::interact). We could add an automatic transparent black fade to all layers behind the top modal to give focus to it.

@plasticartsshow
Copy link
Author

Ok, although this raises a couple of questions (sorry):

  1. should the user not be allowed to pick the fade color/opacity? This would be exceedingly helpful to have for a number of use cases.
  2. As-is, this PR used ui.allocate_response to capture the interaction and tried to completely avoid painting modal contents that weren't showing (which would presumably be the faded lower layers in the modal stack?). Should the lower layers in the stack really be painted at all?

Thanks

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.

Modal dialogs
2 participants