feat!: Implement HasPlaceholder

[mvysny]

Description

Implement the HasPlaceholder interface.

Fixes #4068

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]

Test Results

1 042 files  1 042 suites   1h 13m 43s ⏱️
6 700 tests 6 655 ✔️ 45 💤 0 ❌
7 004 runs  6 948 ✔️ 56 💤 0 ❌

Results for commit 0337d70.

♻️ This comment has been updated with latest results.

[mvysny]

Warning: this PR brings in an incompatible change since it changes the result type of Input.getPlaceholder() from Optional<String> to String. I'd argue that this is OK since:

1. It unifies the return type for Input and for all other components, e.g. TextField
2. placeholder is now again a bean property
3. Returning null will not overwhelmingly cause errors since getPlaceholder() is used sparingly, if at all.

[mcollovati]

Overall LGTM, but I have a couple of questions:

• do we still need to use the property descriptor in the Input element?
• shouldn't also RangeInput implement the new interface?
• should we also update the InputTextElement in flow-html-components-testbench to make it implement com.vaadin.testbench.HasPlaceholder

[mcollovati]

BTW, we should probably create an issue for implementing the testbench RangeInputElement

[mvysny]

do we still need to use the property descriptor in the Input element?

Unfortunately yes, because Input element sets the placeholder attribute, while all other web components and all Has* interfaces set the property. Therefore, we need to override the default implementation in Input.

shouldn't also RangeInput implement the new interface?

Nope, <input type="range"> doesn't support the placeholder attribute as per https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/range

should we also update the InputTextElement in flow-html-components-testbench to make it implement com.vaadin.testbench.HasPlaceholder

Yup, but I can't figure out how to run IT tests in flow-html-components-testbench unfortunately.

BTW, we should probably create an issue for implementing the testbench RangeInputElement

Agree: #18093

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.3.0.alpha6 and is also targeting the upcoming stable 24.3.0 version.