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

Add first run setup dialog #17881

Merged
merged 44 commits into from
Apr 22, 2022
Merged

Add first run setup dialog #17881

merged 44 commits into from
Apr 22, 2022

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Apr 19, 2022

I hope to add more screens to this, but for now I chose one to work with just to get the general structure setup. Quite a few lines of change, but also few modified lines (mostly test and new UI).

Eventually I'd like to convert this to a Screen which displays before the MainMenu but making that work (especially for the user-triggered display case) is a bit hard, so I figured getting the general feel and flow together is a good start. Making it a screen would allow improving the first-import dialog pop (which is a bit weird atm) and also simplify the notification blocking flow, which can best be described as a hack at the moment.

osu.2022-04-19.at.08.50.00.mp4

Follow-up screens will come in separate PRs. This is mostly about the overall flow (and the choice of implementing the UI scale screen first may have been a bad one, as making it work smoothly was the largest part of this implementation).

peppy added 28 commits April 19, 2022 14:53
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skimmed this, nothing major found, left a few comments but they're mostly discretionary. I'm explicitly not leaving comments on the stuff that was pre-empted as temporary (i.e. the exposition of overlay activation state as public)

osu.Game/Overlays/FirstRunSetupOverlay.cs Outdated Show resolved Hide resolved
osu.Game/Overlays/FirstRunSetupOverlay.cs Outdated Show resolved Hide resolved
osu.Game/Overlays/FirstRunSetupOverlay.cs Outdated Show resolved Hide resolved
osu.Game/Overlays/FirstRunSetupOverlay.cs Outdated Show resolved Hide resolved
@peppy
Copy link
Sponsor Member Author

peppy commented Apr 20, 2022

@arflyte can you take a look at the video in the opening post and let me know if you have any existing designs which use the new slanted language that i could draw from to update this to fit in?

@arflyte
Copy link

arflyte commented Apr 20, 2022

I haven't finalised any slanted designs for pop-ups so far. So I think it looks fine for what it is now.

But, if you want some idea, you can look into this file. (Warning, this is just an idea board, so it's ultra god-forbid messy >_<). https://www.figma.com/file/E6kgzLP6CGd62NPSRXLkUX/General-Overlay?node-id=0%3A1

@peppy
Copy link
Sponsor Member Author

peppy commented Apr 20, 2022

I'm going to migrate this to use the newer design language, but the majority of the content should not change so this PR is still in a reviewable state. Will probably come as a separate PR.

@peppy
Copy link
Sponsor Member Author

peppy commented Apr 21, 2022

I've finished updating the design of this to use the new shared logic. If no one has gotten too far in review yet, shall I push directly on this branch? I think it simplifies the code if anything.

Ended up splitting out as there was a bit of extra logic required.

@bdach
Copy link
Collaborator

bdach commented Apr 21, 2022

This seems like a good start 👍 I've merged master here, resolving a trivial conflict along the way, and updated IScreen method signatures in line with framework changes. All changes look simple but please make sure to double check that I haven't broken anything.

@peppy peppy merged commit 89d93ae into ppy:master Apr 22, 2022
@peppy peppy deleted the first-run-setup branch April 22, 2022 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants