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 customizable clips_input property to Control #15358

Closed
wants to merge 1 commit into from

Conversation

Cryszon
Copy link

@Cryszon Cryszon commented Jan 5, 2018

Enables clips_input as a modifiable property to complement clip_contents. See https://github.com/Cryszon/godot-clips-input-example for an example project.

This allows overriding clips_input for GraphEdit and ScrollContainer, but I think the clip_ contents already does the same thing. I used the original clips_input as the name, but was wondering if it should be renamed to clip_input to be more consistent with the existing clip_contents.

This is my first time contributing to Godot so hopefully I'm doing it right. Feedback and guidance is appreciated. I posted this on Discord yesterday but didn't get a reply so I thought I'd go ahead and make the pull request.

@groud
Copy link
Member

groud commented Jan 5, 2018

I would rename clips_input to is_clipping_input for consistency.

@reduz
Copy link
Member

reduz commented Jan 5, 2018 via email

@Cryszon
Copy link
Author

Cryszon commented Jan 5, 2018

Is there another way to make nodes clipped by clip_contents ignore input events? I guess you could do it in GDScript by checking if the nodes are inside the clipping control area, but it seems unnecessarily complex compared to a single flag.

I wouldn't even mind clips_input being automatically set by clip_contents since I don't see any use case where you would need clipped "hidden" nodes to accept input. That's actually how I expected the clip_contents to work in the first place.

@Cryszon
Copy link
Author

Cryszon commented Jan 5, 2018

Also, as far as the use cases go, here's a short video of my example project and what I'm trying to achieve.

@Cryszon
Copy link
Author

Cryszon commented Jan 6, 2018

I agree that it should be connected to clip_contets. I just initially assumed that clip_contents was working as intended and didn't clip input for some weird reason, which is why I added it as a separate property. I totally agree with @reduz that there's no reason to expose it separately.

Should I update this so that clip_contents also sets clips_input and remove the property?

@akien-mga
Copy link
Member

Sorry for the delay, PRs from December/January got piled under a huge backlog due to the 3.0 release...

Should I update this so that clip_contents also sets clips_input and remove the property?

I think it would be worth discussing yes, would you update this PR for that?

@Cryszon
Copy link
Author

Cryszon commented Jul 30, 2018

Sorry I couldn't get back to this during the weekend. Thanks for the fix!

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.

5 participants