Skip to content

Scroll buttons on main menu (issue #793) #880

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.

Sign up for GitHub

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

Jump to bottom
Merged
SnipUndercover merged 1 commit into from
Dec 3, 2025
Merged

Scroll buttons on main menu (issue #793) #880

SnipUndercover merged 1 commit into from
Dec 3, 2025

Conversation

@DashingCat
Copy link
Contributor

@DashingCat DashingCat commented Feb 20, 2025

Allow buttons to scroll in the main menu so the currently selected button is always on screen.

Closes #793.

@maddie480 maddie480 added the 1: review needed This PR needs 2 approvals to be merged (bot-managed) label May 18, 2025
SnipUndercover
SnipUndercover reviewed Nov 28, 2025
View reviewed changes
Copy link
Member

@SnipUndercover SnipUndercover left a comment

Choose a reason for hiding this comment

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

Looks good, I just want to test this myself before approving.

@SnipUndercover SnipUndercover self-assigned this Nov 28, 2025
Wartori54
Wartori54 requested changes Nov 29, 2025
View reviewed changes
Copy link
Member

@Wartori54 Wartori54 left a comment

Choose a reason for hiding this comment

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

It works, yet it doesn't feel right, the scrolling is not smooth at all and breaks the style of Celeste menus. I believe adding the same smoothing on scroll that text menus have would work nicely and fit perfectly, if that's added that's an immediate LGTM on my end.
But I know this may be asking too much, so I will ask, instead, for a TODO to get added in the relevant part asking for someone to implement smooth scrolling in the future.

@DashingCat DashingCat force-pushed the 793-scroll_buttons_on_main_menu branch from 7f1fbbf to 07aaf62 Compare November 29, 2025 23:43
@DashingCat
Copy link
Contributor Author

DashingCat commented Nov 29, 2025

Thanks @Wartori54 for the review, I added a TODO mentioning smooth scrolling.

@DashingCat DashingCat requested a review from Wartori54 November 29, 2025 23:46
@maddie480-bot maddie480-bot added 2: changes requested This PR cannot be merged because changes were requested (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Nov 29, 2025
@maddie480-bot
Copy link
Member

maddie480-bot commented Nov 30, 2025

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Dec 3, 2025, 12:23 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot maddie480-bot added 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) and removed 2: changes requested This PR cannot be merged because changes were requested (bot-managed) labels Nov 30, 2025
@maddie480-bot
Copy link
Member

maddie480-bot commented Dec 3, 2025

The last-call window for this pull request ended. It can now be merged if no blockers were brought up.

@maddie480-bot maddie480-bot added 4: ready to merge This PR was approved and the last-call window is over (bot-managed) and removed 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) labels Dec 3, 2025
@SnipUndercover SnipUndercover merged commit 6517308 into EverestAPI:dev Dec 3, 2025
3 checks passed
@DashingCat DashingCat mentioned this pull request Dec 28, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SnipUndercover SnipUndercover SnipUndercover left review comments

@microlith57 microlith57 microlith57 approved these changes

@Wartori54 Wartori54 Wartori54 approved these changes

Assignees

@SnipUndercover SnipUndercover

Labels

4: ready to merge This PR was approved and the last-call window is over (bot-managed)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Scrollable main menu buttons on Vanilla main menu mode

6 participants

@DashingCat @maddie480-bot @microlith57 @SnipUndercover @Wartori54 @maddie480