feat: Find a parent component with a given type

[mstahv]

Description

Add a helper method to seek the parent component of given type from the component tree.

Fixes #13984

Type of change

• Bugfix
• Feature

Checklist

• I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
• I have added a description following the guideline.
• The issue is created in the corresponding repository and I have referenced it.
• I have added tests to ensure my change is effective and works as intended.
• New and existing tests are passing locally with my change.
• I have performed self-review and corrected misspellings.

Additional for Feature type of change

• Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

[github-actions]

Unit Test Results

   980 files     980 suites   51m 8s ⏱️
6 316 tests 6 268 ✔️ 48 💤 0 ❌
6 578 runs  6 524 ✔️ 54 💤 0 ❌

Results for commit 7c24416.

♻️ This comment has been updated with latest results.

[mcollovati]

Personally I don't like that much the idea of traversing the tree and directly interact with an ancestor component, but I can get the points expressed in the related issue.
In addition we do not prevent this in any way and I'm pretty sure many developer may have already implemented such a helper on their own projects.

Some cosiderations on the proposed implementation:

• I would maybe move the implementation to ComponentUtil class since we already have some traversal helpers in that class.
• If instead we want to have it in Component, I think the API should not be public. I see it more as an internal helper for the component, rather than a way to traverse tree from outside.
• Do not restrict the component type to Component make sense to me; it is implicit that we will find a Component, but it may be useful to search it for example by a mixin or a custom interface.
• I would maybe call the method findAncestor rather than findParent

[mstahv]

ComponentUtil feels like a set of internal helpers to me and it is not documented at all (other than JavaDocs), so I think far less developers would find it from there.

Renamed. Much better!

Also make protected. This is bit controversial. I guess most often one uses that from component, so protected should be fine, but somehow I have a feeling that someday I find my self opening that method directly as public in some Vaadin view 🤷‍♂️

[mcollovati]

I would still move the implementation to ComponentUtil and then have the method in Component that delegates to it.
But it is up to you.
Other than that, LGTM

[mshabarov]

I would still move the implementation to ComponentUtil and then have the method in Component that delegates to it.

This sounds better for me as well.
By the way, I don't get the idea why you made it protected? The use case is to get the layout/menu or other high level component to modify it somehow. This method is invoked for some child component, so should the application actually extend the component to use this API, since is a protected, not public.
I would make it public, but transfer the implementation to the utils.

[mcollovati]

I suggested to make the method protected

I see it more as an internal helper for the component, rather than a way to traverse tree from outside

But it seems that I'm the only one that do not see the usage from outside a component, so let it be public 😄

[mstahv]

Reverted back to public.

Are we sure about having the actual implementation delegated to ComponentUtil 🤔 Then we would have basically the same method in two places. Would somebody want to have it in ComponentUtil if it is available as a public instance method 🤔

[mshabarov]

Well, I would say it can be reused when moved to utils. But it can also be reused if placed to component directly.

[mstahv]

👌 I'll write some basic unit test for this and convert to actual PR.

[mstahv]

@mshabarov I'm ready with this!

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
0.0% Duplication

[vaadin-bot]

This ticket/PR has been released with Vaadin 23.2.0.