Refactor report popovers to use dialogs#38337
Draft
LiquidPL wants to merge 7 commits into
Draft
Conversation
Used to add a loading overlay to the report dialog.
This is needed for the user report form, which has a confirmation message directly in the dialog, as opposed to other forms, which display the confirmation alongside the reported content.
As mentioned in ppy#37647 (comment).
Previously it would set the bindable to `null`, which was breaking the report forms expecting the reason field to be an empty string by default.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #37638 and #37647.
This PR takes a different approach from the previous PR series, by redesigning the report forms to fit well in the
PopupDialog, as well as making it use the new form controls.ReportPopover -> ReportDialog
Just a bulk rename of all related classes into their new names to avoid hassle while reviewing. Notably the new names are incorrect, since all the report forms are still popovers despite being prefixed with
Dialog.Override Content on PopupDialog
Used to add a loading overlay to the report dialog.
Add option to not hide dialog before action invoke
This is needed for the user report form, which has a confirmation message directly in the dialog, as opposed to other forms, which display the confirmation alongside the reported content.
It's something I'm the least confident about when it comes to not breaking something somewhere with other dialogs.
Add dim layer to DialogOverlay
As mentioned in #37647 (comment). Necessary since the report dialogs will be displayed over other overlays, which are already making use of the gamewide dim layer.
Set default on FormTextBox bindable
FormTextBox'sCurrentbindable did not have a default empty value set, causing report API requests to fail because the report reason field was suddenlynullinstead of a string.Refactor report popovers to use dialogs
The main commit of this pull request.
Some demos:
Screencast_20260717_193633.webm
Screencast_20260717_193751.webm
Screencast_20260717_193819.webm
SettingsNotefor this since it looks much better than just a plain red text sprite, let me know if I should revert this)