Skip to content

DialogHost API revamp #3841

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

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

Conversation

corvinsz
Copy link
Member

Initially I "just" wanted to implement a fullscreen dialog, but I asked myself "why stop here?"

Here are a few thoughts/changes to explain this PR:

  • To not clutter the existing Show methods even more I opted to implement a new class called DialogOptions, to have this option-based configuration-style
  • DialogOptions has alot of Previous<some property> properties, to keep it as compatible with the old Show methods. The way this works is by storing the corresponding property before setting it to something different, then show the dialog, and then revert all the properties to the previously stored value. While the old and new API could be used interchangingly, it is probably a good idea to just stick with one of the two.
  • I added a default CloseButton (PART_CloseButton) to the template
  • I added a new interface IDialogService. We already have an abstraction for the Snackbar (ISnackbarMessageQueue), but not (yet) for the DialogHost
  • Also I replaced alot of code with syntactic sugar or newer syntaxes

There are 2 things I want to publicly address

  1. What other Show methods should we add to the IDialogService?
  2. I for the life of me can't figure out why the dialog is not sizing to fullscreen if I press the maximize button. I have been trying to figure this out for hours now without any luck:
    fullscreenDialogProblem

There are still some ToDos left, but I wanted to share this publicly now so any ideas or recommendations can be incorporated.

If all of this is total nonsense, lmk 😆

Corvin Szimion and others added 30 commits October 24, 2024 10:55
-adjust the Popups size depending on the "isFullscreenDialog"
-removed "IsFullscreenDialog" from old "Show" API
-added new generic "Show" API which accepts DialogOptions
-added Apply and Revert methods for DialogOptions
…width of the dialog can be restored correctly
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.

1 participant