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

Enable Scrolling signal when scrolling with middle mouse on RichTextLabel or ScrollContainer #90988

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

TheSofox
Copy link
Contributor

Fixes #46421

Added new function to ScrollBar so that the bar could be scrolled in a way that would emit a scrolling signal. Edited RichTextLabel and ScrollContainer to use such functionality so scrolling either using the mouse scrollwheel would cause the signal to be emitted from the ScrollBar

@TheSofox TheSofox requested a review from a team as a code owner April 21, 2024 18:13
@AThousandShips AThousandShips added bug topic:gui cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Apr 21, 2024
@AThousandShips AThousandShips added this to the 4.3 milestone Apr 21, 2024
@TheSofox TheSofox force-pushed the scrolling-signal-fix branch from c36b87b to 9dbf384 Compare April 22, 2024 11:15
@TheSofox
Copy link
Contributor Author

TheSofox commented Apr 22, 2024

Amended PR. Added scroll_to and made it so scrolling signal is only emitted if scroll or scroll_to function is called. Updated various function calls to use these functions so that signal continues to be emitted by their actions.

I feel this is more clear and consistent behaviour.

@TheSofox TheSofox force-pushed the scrolling-signal-fix branch from 9dbf384 to fd43eb8 Compare April 22, 2024 11:24
@AThousandShips AThousandShips changed the title Enabled Scrolling signal when scrolling with middle mouse on RichTextLabel or ScrollContainer Enable Scrolling signal when scrolling with middle mouse on RichTextLabel or ScrollContainer Apr 22, 2024
scene/gui/scroll_bar.h Outdated Show resolved Hide resolved
@TheSofox TheSofox force-pushed the scrolling-signal-fix branch from fd43eb8 to 3aec2cc Compare April 22, 2024 13:52
@KoBeWi
Copy link
Member

KoBeWi commented Apr 22, 2024

The scroll signal is not emitted when dragging.

scene/gui/scroll_bar.cpp Outdated Show resolved Hide resolved
@KoBeWi
Copy link
Member

KoBeWi commented Apr 22, 2024

ScrollContainer has some set_value()s from touch events that are not changed to the new method. Is this intended?

@TheSofox
Copy link
Contributor Author

ScrollContainer has some set_value()s from touch events that are not changed to the new method. Is this intended?

Guess I was unsure about touch events, since they kinda do a bit of momentum and physics rather than direct physical interaction, but I suppose it's best to change them.

@TheSofox TheSofox force-pushed the scrolling-signal-fix branch from 3aec2cc to d68352b Compare April 22, 2024 17:14
@KoBeWi
Copy link
Member

KoBeWi commented Apr 22, 2024

There seems to be another bug. When you scroll continuously using mouse wheel, the signal will be emitted even if no scrolling happens. EDIT: #90988 (comment)

…xtLabel/ScrollContainer
@TheSofox TheSofox force-pushed the scrolling-signal-fix branch from d68352b to 30356a4 Compare April 22, 2024 19:44
@TheSofox
Copy link
Contributor Author

There seems to be another bug. When you scroll continuously using mouse wheel, the signal will be emitted even if no scrolling happens. EDIT: #90988 (comment)

Fixed.

@akien-mga akien-mga merged commit 7abe0c6 into godotengine:master Apr 22, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release topic:gui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scrolling signal on RichTextLabel/ScrollContainer's scrollbar doesn't work when scrolling with scroll wheel
4 participants