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

Expose horizontal/vertical custom_step as editor property for the ScrollContainer #70868

Merged

Conversation

Maran23
Copy link
Contributor

@Maran23 Maran23 commented Jan 3, 2023

Closes: godotengine/godot-proposals#6038

The horizontal and vertical custom_step can now be set via the editor.
image

@Maran23 Maran23 requested review from a team as code owners January 3, 2023 09:44
@YuriSizov YuriSizov added this to the 4.x milestone Jan 3, 2023
@Maran23 Maran23 force-pushed the 4-x-scrollcontainer-custom-step-editor branch 2 times, most recently from 4d6924d to 91cca89 Compare January 3, 2023 11:47
@@ -54,9 +54,15 @@
<member name="scroll_horizontal" type="int" setter="set_h_scroll" getter="get_h_scroll" default="0">
The current horizontal scroll value.
</member>
<member name="scroll_horizontal_custom_step" type="float" setter="set_horizontal_custom_step" getter="get_horizontal_custom_step" default="-1.0">
Overrides the step used when clicking the horizontal increment and decrement buttons or when using arrow keys when the [ScrollBar] is focused.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Overrides the step used when clicking the horizontal increment and decrement buttons or when using arrow keys when the [ScrollBar] is focused.
Overrides the step used when clicking the scroll bar's horizontal increment and decrement buttons or when using arrow keys when the [ScrollBar] is focused.

The buttons are not part of the container, so it's better to clarify that.
Same below.

Copy link
Contributor Author

@Maran23 Maran23 Apr 29, 2023

Choose a reason for hiding this comment

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

Good idea. Should I adapt the description of the class ScrollBar then too?
It has nearly the same description:
Overrides the step used when clicking increment and decrement buttons or when using arrow keys when the [ScrollBar] is focused.

Edit: I can also do that in a new PR. I think it makes sense to keep both custom_step descriptions in sync. :)

Copy link
Member

@KoBeWi KoBeWi Apr 29, 2023

Choose a reason for hiding this comment

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

No, these descriptions shouldn't be in sync. But ScrollBar's description of custom step could be improved.

ScrollBar's description mentions increment/decrement buttons, but they don't exist in the default theme. I think it could be clarified that you need to define their icons in the theme to make the buttons appear.

ScrollContainer uses ScrollBars as internal node and the buttons are part of ScrollBars, not ScrollContainer (I mean not directly). Its description could link to custom step in ScrollBar class, like

Overrides the [member ScrollBar.custom_step] of the internal scroll bar.

Copy link
Contributor Author

@Maran23 Maran23 Apr 29, 2023

Choose a reason for hiding this comment

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

But ScrollBar's description of custom step could be improved.

Agree. What I mean is that it might be good to document in ScrollBar that the increment/decrement buttons are also internal, or at least kind of. Maybe a good idea for another PR though.

I adapted the doc changes. :)

@Maran23 Maran23 force-pushed the 4-x-scrollcontainer-custom-step-editor branch from 91cca89 to 29358da Compare April 29, 2023 21:58
@akien-mga akien-mga modified the milestones: 4.x, 4.1 May 8, 2023
@akien-mga akien-mga merged commit 704560c into godotengine:master May 8, 2023
@akien-mga
Copy link
Member

Thanks!

@Maran23 Maran23 deleted the 4-x-scrollcontainer-custom-step-editor branch August 4, 2023 18:03
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.

[ScrollContainer] Expose horizontal/vertical custom_step of internal HScrollBar and VScrollBar.
4 participants