feat!: NativeLabel.setFor() lazily resolves and auto-generates IDs#24425
feat!: NativeLabel.setFor() lazily resolves and auto-generates IDs#24425Artur- wants to merge 4 commits into
Conversation
The setFor(Component) method now uses beforeClientResponse to lazily resolve the component's ID at sync time rather than requiring it immediately. If the component still doesn't have an ID by then, one is automatically generated. This makes NativeLabel easier to use because developers no longer need to manually assign IDs to components when using setFor(). Fixes #24424 https://claude.ai/code/session_014v3s2ioBp5pSwd4F3Euqt5
|
Same pattern should be applied also for setAriaLabelledBy(String ariaLabelledBy) (in HasAriaLabel). Check if there could be a common helper. |
Adds a Component overload of setAriaLabelledBy that lazily resolves the target component's id at sync time and auto-generates one if needed, mirroring the pattern introduced for NativeLabel.setFor(Component). The lazy-resolve-and-auto-generate logic is extracted into a new ComponentUtil.resolveOrGenerateIdLater helper and reused by both NativeLabel.setFor(Component) and HasAriaLabel.setAriaLabelledBy(Component). Note: as with NativeLabel.setFor, the new Component overload makes setAriaLabelledBy(null) ambiguous; callers clearing the attribute must cast to (String) null.
|
Now Claude should seek for other similar patterns in the framework (and in flow-componens project) that I have missed/don't remember. A thing to also consider: deprecate the current versions that work with string identifier. I can only see those relevant/better for some integrations with other frameworks/libraries and then Element API level of integration might be even better. |
HtmlComponentSmokeTest.testSetter requires each setter to have a getter of the matching type. The new Component overload doesn't (getter still returns Optional<String>), mirroring NativeLabel.setFor(Component) which already has an isSpecialSetter exclusion for the same reason.
|
| * @param labelComponent | ||
| * the component to use as the label, not {@code null} | ||
| */ | ||
| default void setAriaLabelledBy(Component labelComponent) { |
There was a problem hiding this comment.
The introduction of this method is potentially a breaking change that might cause compilation errors if the application used to call setAriaLabelledBy(String) with null as an argument.
Perhaps it's fine but should be documented in the release notes.
There was a problem hiding this comment.
Somebody would in this case be clearing the "binding" to a label component. This is maybe one of those cases that it can happen even if we don't believe it. My hunch is this would be extremely rare (and easy to fix), so I'd just do that release note comment and follow the situation. If the world explodes in e.g. ecosystem build or otherwise during beta period, we can rollback or rename to e.g. setAriaLabelledByComponent.
mcollovati
left a comment
mcollovati
left a comment
There was a problem hiding this comment.
Overall looks good. There's only the potential breaking change introduced by the new setAriaLabelledBy method.
mcollovati
changed the title
5 participants
The setFor(Component) method now uses beforeClientResponse to lazily resolve the component's ID at sync time rather than requiring it immediately. If the component still doesn't have an ID by then, one is automatically generated.
This makes NativeLabel easier to use because developers no longer need to manually assign IDs to components when using setFor().
Fixes #24424
https://claude.ai/code/session_014v3s2ioBp5pSwd4F3Euqt5