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

Update beat snap divisor control to be more in line with new designs #18393

Merged
merged 6 commits into from
May 25, 2022

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented May 24, 2022

Note that this doesn't match figma 100%. This is just focused on getting it to fit with the surrounding controls for now, proportionally and corner radius-ly.

@smoogipoo
Copy link
Contributor

Can you show how this actually looks in BeatDivisorControl? Personally find this kinda weird since the sizing is arbitrary and completely tailored to the timeline display.

@peppy
Copy link
Sponsor Member Author

peppy commented May 24, 2022

The intended use is for the designs as per figma:

Figma 2022-05-24 at 09 20 23

But I do understand your point. I figured that we would want these ratios available, even if that are applied at different multipliers in each location.

osu Game Tests 2022-05-24 at 09 21 13

This is how it currently looks. Not 100% happy but it's something.

@pull-request-size pull-request-size bot added size/L and removed size/M labels May 24, 2022
@peppy
Copy link
Sponsor Member Author

peppy commented May 24, 2022

I've pushed the actual design to this PR as it doesn't make much sense to leave it stand-alone at this point. Best consume as a whole.

@peppy peppy changed the title Move beat divisor tick size retrieval to static methods Update beat snap divisor control to be more in line with new designs May 24, 2022
smoogipoo
smoogipoo previously approved these changes May 24, 2022
Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

I'm gonna cover my eyes for this one because I'm not sure it makes much sense in either design.

@bdach
Copy link
Collaborator

bdach commented May 24, 2022

I've pushed a change to cache some more OverlayColourProviders to fix tests. Looks ok otherwise.

@smoogipoo smoogipoo merged commit 8ca6f21 into ppy:master May 25, 2022
@peppy peppy deleted the editor-beat-divisor-size-static branch May 25, 2022 03:55
@peppy peppy mentioned this pull request May 25, 2022
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.

3 participants