Add Vaadin 25 support while maintaining Vaadin 24 compatibility#27
Add Vaadin 25 support while maintaining Vaadin 24 compatibility#27
Conversation
Update JavaScript calls in classes ReplaceableLoginOverlay and LoginLayout to support Vaadin 24 & Vaadin 25. Close #23
Use demo source condition attribute to select between Vaadin 24 & Vaadin 25 styles.
Add testbench-rpc denpendency to be able to retrieve Vaadin version.
WalkthroughThis PR refactors inline JavaScript snippets in the login overlay implementation into centralized utility methods for improved maintainability, introduces version-aware support for Vaadin 25+, adds new convenience methods to the ReplaceableLoginOverlay interface, and updates test infrastructure to handle multiple Vaadin versions through conditional logic and RPC support. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/ServerVersionCallables.java (1)
20-29: Move this contract out of the.itpackage.Both
TestExtendedLoginOverlayViewandTestLoginLayoutimplement this interface from outsidecom.flowingcode.vaadin.addons.extendedlogin.it, so the current location makes the server-side fixtures depend on the integration-test namespace. A neutral test-support package would keep that dependency direction cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/ServerVersionCallables.java` around lines 20 - 29, Move the ServerVersionCallables interface out of the com.flowingcode.vaadin.addons.extendedlogin.it package into a neutral test-support package (e.g., com.flowingcode.vaadin.addons.extendedlogin.testsupport) so integration tests don't become a dependency target; update the package declaration in the ServerVersionCallables file and adjust imports in classes that implement it (TestExtendedLoginOverlayView, TestLoginLayout) to reference the new package name, and run/compile tests to ensure no remaining references to the old .it package remain.src/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/LoginLayoutIT.java (1)
57-62: Redundant conditional after assertTrue.The
if (h2exists)block on line 59 is unreachable ifh2existsis false becauseassertTrueon line 58 would already fail the test.Simplify by removing redundant conditional
} else { - boolean h2exists = vlfw.$(H2Element.class).exists(); - assertTrue("H2 is not present", h2exists); - if (h2exists) { - assertEquals("Change Password", vlfw.$(H2Element.class).first().getText()); - } + assertTrue("H2 is not present", vlfw.$(H2Element.class).exists()); + assertEquals("Change Password", vlfw.$(H2Element.class).first().getText()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/LoginLayoutIT.java` around lines 57 - 62, The test uses a redundant if (h2exists) after assertTrue("H2 is not present", h2exists) in LoginLayoutIT.java; remove the conditional and directly call assertEquals("Change Password", vlfw.$(H2Element.class).first().getText()) after the assertTrue (or replace both with a single assertEquals if you prefer) so the check of vlfw.$(H2Element.class).first().getText() is executed only when the existence assertion has guaranteed the element is present.src/main/java/com/flowingcode/vaadin/addons/extendedlogin/ReplaceableLoginOverlay.java (1)
55-78: Potential race condition with sequential asynchronous calls.
replaceHeaderComponentissues two separateexecuteJscalls, each wrapped insetTimeoutby the utility method. Without a guaranteed execution order, the append operation could theoretically execute before the clear operation completes, though in practice browsers typically execute same-ticksetTimeout(..., 0)callbacks in order.Consider combining the operations into a single script to eliminate any timing ambiguity.
Combine clear and append into single script
default void replaceHeaderComponent(HasElement withElement) { getElement().appendChild(withElement.getElement()); - this.getElement() .executeJs( LoginOverlayUtils.getOverlayWrapperScript( """ var brand = overlayWrapper.shadowRoot ? overlayWrapper.shadowRoot.querySelector('[part="brand"]') : overlayWrapper.querySelector('[part="brand"]'); if (brand) { brand.replaceChildren(); - } - """)); - - this.getElement() - .executeJs( - LoginOverlayUtils.getOverlayWrapperScript( - """ - var brand = overlayWrapper.shadowRoot ? overlayWrapper.shadowRoot.querySelector('[part="brand"]') : overlayWrapper.querySelector('[part="brand"]'); - if (brand) { brand.appendChild($0); } """), withElement); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/flowingcode/vaadin/addons/extendedlogin/ReplaceableLoginOverlay.java` around lines 55 - 78, The replaceHeaderComponent method issues two separate executeJs calls which can race; change it to a single executeJs call using LoginOverlayUtils.getOverlayWrapperScript that runs one script which finds the [part="brand"], clears its children and then appends $0 in the same tick, passing withElement as the single argument so both clear-and-append happen atomically; update the method to remove the two executeJs invocations and use one combined script invocation referencing withElement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pom.xml`:
- Around line 467-470: The v25 Maven profile currently sets
<vaadin.version>25.0.6</vaadin.version> but leaves
<jetty.version>11.0.26</jetty.version>, which is incompatible; update the v25
profile to bump the jetty.version property to a Jetty 12.x release (e.g.,
12.1.x) so Jetty supports Jakarta Servlet 6.1+ required by Vaadin 25 — locate
the jetty.version property in the v25 profile and change its value to a 12.1.x
version.
In
`@src/main/java/com/flowingcode/vaadin/addons/extendedlogin/LoginOverlayUtils.java`:
- Around line 61-75: The getFormWrapperScript method fails to traverse the
overlay's shadow DOM when locating the <form>, so update the lookup for the form
on the overlayFormWrapper to check overlayFormWrapper.shadowRoot first (like
getOverlayWrapperScript/getLoginFormWrapperScript): if
overlayFormWrapper.shadowRoot exists, query the form from
overlayFormWrapper.shadowRoot.querySelector('form'), otherwise fall back to
overlayFormWrapper.querySelector('form'), then run the formatted action when
form is found (symbols: getFormWrapperScript, overlayFormWrapper, shadowRoot,
querySelector).
In
`@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestLoginLayout.java`:
- Around line 62-68: The preprocessor comment markers in TestLoginLayout's
getMajorVersion method use a spaced form ("// `#if`" and "// `#endif`") inconsistent
with the rest of the codebase; update the conditional block around the
getMajorVersion method (class TestLoginLayout, method getMajorVersion) to use
the no-space form ("//#if" and "//#endif") so the preprocessor recognizes the
directives consistently.
In
`@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestLoginLayoutView.java`:
- Around line 76-83: The onAccept() handler currently treats two empty strings
as a valid password change; update onAccept() to reject blank or whitespace-only
passwords before showing success by ensuring password and confirmPassword are
non-null and not blank (e.g., check password != null &&
!password.trim().isEmpty() and same for confirmPassword) and only then compare
equality (password.equals(confirmPassword)) to show "Password changed."
otherwise show the mismatch/empty error; refer to the onAccept(), password and
confirmPassword variables for locating the change.
- Around line 41-45: The two `@CssImport` annotations in TestLoginLayoutView (the
lines with `@CssImport`("./styles/extended-login-styles.css") and
`@CssImport`("./styles/extended-login-styles-v25.css")) are both compiled because
the //#if comments are not a preprocessing step; remove both annotations and
instead load the appropriate stylesheet at runtime in the view's constructor (or
an init method) by checking the Vaadin version or a config flag and calling
UI.getCurrent().getPage().addStyleSheet(...) or
UI.getCurrent().getElement().executeJs(...) to include the correct CSS file;
this ensures only the version-specific stylesheet is loaded instead of relying
on compile-time comments.
In `@src/test/resources/META-INF/frontend/styles/extended-login-styles-v25.css`:
- Around line 1-20: The CSS license header starts with the unusual "/*-" and
lacks the required whitespace, triggering Stylelint's comment-whitespace-inside
rule for the file extended-login-styles-v25.css; fix by either (A) normalizing
the header comments in that file: change the opening "/*-" to a standard "/*"
and ensure there is a space immediately after "/*" and immediately before the
closing "*/" (so the license block and the "/*Demo styles*/" comment comply), or
(B) add an override in your stylelint config to whitelist this generated license
header by disabling or ignoring comment-whitespace-inside for that file (use an
override for the filename pattern and set the rule to null or exempt the header
pattern).
---
Nitpick comments:
In
`@src/main/java/com/flowingcode/vaadin/addons/extendedlogin/ReplaceableLoginOverlay.java`:
- Around line 55-78: The replaceHeaderComponent method issues two separate
executeJs calls which can race; change it to a single executeJs call using
LoginOverlayUtils.getOverlayWrapperScript that runs one script which finds the
[part="brand"], clears its children and then appends $0 in the same tick,
passing withElement as the single argument so both clear-and-append happen
atomically; update the method to remove the two executeJs invocations and use
one combined script invocation referencing withElement.
In
`@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/LoginLayoutIT.java`:
- Around line 57-62: The test uses a redundant if (h2exists) after
assertTrue("H2 is not present", h2exists) in LoginLayoutIT.java; remove the
conditional and directly call assertEquals("Change Password",
vlfw.$(H2Element.class).first().getText()) after the assertTrue (or replace both
with a single assertEquals if you prefer) so the check of
vlfw.$(H2Element.class).first().getText() is executed only when the existence
assertion has guaranteed the element is present.
In
`@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/ServerVersionCallables.java`:
- Around line 20-29: Move the ServerVersionCallables interface out of the
com.flowingcode.vaadin.addons.extendedlogin.it package into a neutral
test-support package (e.g.,
com.flowingcode.vaadin.addons.extendedlogin.testsupport) so integration tests
don't become a dependency target; update the package declaration in the
ServerVersionCallables file and adjust imports in classes that implement it
(TestExtendedLoginOverlayView, TestLoginLayout) to reference the new package
name, and run/compile tests to ensure no remaining references to the old .it
package remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33ac540c-6001-4b01-9627-752c3e8dd146
📒 Files selected for processing (15)
pom.xmlsrc/main/java/com/flowingcode/vaadin/addons/extendedlogin/LoginLayout.javasrc/main/java/com/flowingcode/vaadin/addons/extendedlogin/LoginOverlayUtils.javasrc/main/java/com/flowingcode/vaadin/addons/extendedlogin/ReplaceableLoginOverlay.javasrc/test/java/com/flowingcode/vaadin/addons/AppShellConfiguratorImpl.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/ExtendedLoginOverlayDemo.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/LoginLayoutDemo.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestExtendedLoginOverlayView.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestLoginLayout.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestLoginLayoutView.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/AbstractViewTest.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/ExtendedLoginOverlayIT.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/LoginLayoutIT.javasrc/test/java/com/flowingcode/vaadin/addons/extendedlogin/it/ServerVersionCallables.javasrc/test/resources/META-INF/frontend/styles/extended-login-styles-v25.css
| static String getFormWrapperScript(String action) { | ||
| return """ | ||
| setTimeout(() => { | ||
| var overlayFormWrapper = document.getElementById('vaadinLoginOverlayWrapper'); | ||
| if (!overlayFormWrapper) { | ||
| overlayFormWrapper = document.querySelector('vaadin-login-overlay'); | ||
| } | ||
| if (!overlayFormWrapper) return; | ||
| var form = overlayFormWrapper.querySelector('form'); | ||
| if (form) { | ||
| %s | ||
| } | ||
| }); | ||
| """.formatted(action); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Vaadin 25 vaadin-login-overlay shadow DOM structure form element location
💡 Result:
In Vaadin 25, the <vaadin-login-overlay> contains its UI inside its shadow root, and it exposes the key nodes via shadow parts, including:
form-wrapper(wrapper around the login form)form(the login form element/area) (cdn.vaadin.com)
However, the actual <vaadin-login-form> component is designed to have no shadowRoot so the underlying native <form> + inputs stay accessible at the document layer (important for password managers). Using <vaadin-login-overlay> is specifically meant to ensure the login form is attached to document body via the overlay mechanism. (cdn.vaadin.com)
Practical DOM “where is the form?” pointers
-
Styling hook (recommended):
vaadin-login-overlay::part(form)targets the form area exposed by the overlay. (cdn.vaadin.com)
-
JS query (shadow DOM path to the exposed form area):
document.querySelector('vaadin-login-overlay')?.shadowRoot?.querySelector('[part="form"]')(cdn.vaadin.com)
-
If you specifically need the native
<form>tag (e.g., for testing):- Look under the rendered
<vaadin-login-form>instance (which itself has no shadowRoot), after the overlay is opened/attached todocument.body. (cdn.vaadin.com)
- Look under the rendered
Citations:
- 1: https://cdn.vaadin.com/vaadin-web-components/25.0.2/elements/vaadin-login-overlay/
- 2: https://cdn.vaadin.com/vaadin-web-components/25.0.0-beta6/elements/vaadin-login-form/?utm_source=openai
- 3: https://cdn.vaadin.com/vaadin-web-components/25.0.2/elements/vaadin-login-overlay/
- 4: https://cdn.vaadin.com/vaadin-web-components/25.0.2/elements/vaadin-login-overlay/
- 5: https://cdn.vaadin.com/vaadin-web-components/25.0.0-beta6/elements/vaadin-login-form/?utm_source=openai
🏁 Script executed:
cat -n src/main/java/com/flowingcode/vaadin/addons/extendedlogin/LoginOverlayUtils.javaRepository: FlowingCode/ExtendedLoginAddon
Length of output: 4432
Inconsistent shadow DOM traversal in getFormWrapperScript for Vaadin 25 compatibility.
This method does not check the shadowRoot of <vaadin-login-overlay> when querying for the form element, unlike getOverlayWrapperScript (lines 41–43) and getLoginFormWrapperScript (lines 92–94). In Vaadin 25, the form resides within the overlay's shadow DOM, so calling overlayFormWrapper.querySelector('form') at line 69 will return null and the action will not execute.
To match the pattern used in the other methods, check overlayFormWrapper.shadowRoot before querying for the form element.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/flowingcode/vaadin/addons/extendedlogin/LoginOverlayUtils.java`
around lines 61 - 75, The getFormWrapperScript method fails to traverse the
overlay's shadow DOM when locating the <form>, so update the lookup for the form
on the overlayFormWrapper to check overlayFormWrapper.shadowRoot first (like
getOverlayWrapperScript/getLoginFormWrapperScript): if
overlayFormWrapper.shadowRoot exists, query the form from
overlayFormWrapper.shadowRoot.querySelector('form'), otherwise fall back to
overlayFormWrapper.querySelector('form'), then run the formatted action when
form is found (symbols: getFormWrapperScript, overlayFormWrapper, shadowRoot,
querySelector).
| // #if vaadin eq 0 | ||
| @Override | ||
| @ClientCallable | ||
| public int getMajorVersion() { | ||
| return com.vaadin.flow.server.Version.getMajorVersion(); | ||
| } | ||
| // #endif |
There was a problem hiding this comment.
Preprocessor directive format inconsistency.
Line 62 uses // #if`` (with a space) while other files use //#if (no space). If the preprocessor is strict about syntax, this could cause the conditional block to be treated incorrectly.
Proposed fix for consistency
- // `#if` vaadin eq 0
+ //#if vaadin eq 0
`@Override`
`@ClientCallable`
public int getMajorVersion() {
return com.vaadin.flow.server.Version.getMajorVersion();
}
- // `#endif`
+ //#endif📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // #if vaadin eq 0 | |
| @Override | |
| @ClientCallable | |
| public int getMajorVersion() { | |
| return com.vaadin.flow.server.Version.getMajorVersion(); | |
| } | |
| // #endif | |
| //#if vaadin eq 0 | |
| `@Override` | |
| `@ClientCallable` | |
| public int getMajorVersion() { | |
| return com.vaadin.flow.server.Version.getMajorVersion(); | |
| } | |
| //#endif |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestLoginLayout.java`
around lines 62 - 68, The preprocessor comment markers in TestLoginLayout's
getMajorVersion method use a spaced form ("// `#if`" and "// `#endif`") inconsistent
with the rest of the codebase; update the conditional block around the
getMajorVersion method (class TestLoginLayout, method getMajorVersion) to use
the no-space form ("//#if" and "//#endif") so the preprocessor recognizes the
directives consistently.
There was a problem hiding this comment.
It is not something to worry about
There was a problem hiding this comment.
@paodb, understood! I'll drop that concern. Thanks for clarifying.
| //#if vaadin eq 24 | ||
| @CssImport("./styles/extended-login-styles.css") | ||
| //#else | ||
| @CssImport("./styles/extended-login-styles-v25.css") | ||
| //#endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking for source-condition directives:"
rg -n -C2 '//#(if|else|endif)\b'
echo
echo "Checking Maven config for preprocessing/filtering of Java sources:"
fd 'pom.xml' -x rg -n -C3 '//#if|vaadin eq|source condition|preprocess|templating|filtering|replacer|maven-resources-plugin|maven-antrun-plugin'Repository: FlowingCode/ExtendedLoginAddon
Length of output: 6675
🏁 Script executed:
cd /tmp && cat pom.xml | sed -n '380,450p'Repository: FlowingCode/ExtendedLoginAddon
Length of output: 113
🏁 Script executed:
fd 'pom.xml' -x head -n 500 {} | tail -n +380 | head -n 70Repository: FlowingCode/ExtendedLoginAddon
Length of output: 1936
🏁 Script executed:
rg -A 30 'maven-resources-plugin' pom.xmlRepository: FlowingCode/ExtendedLoginAddon
Length of output: 1012
🏁 Script executed:
rg 'maven-replacer|maven-antrun|source-condition|maven-shade|replacer|antrun' pom.xmlRepository: FlowingCode/ExtendedLoginAddon
Length of output: 56
🏁 Script executed:
rg -i 'preprocess|filter|condition' pom.xml | head -30Repository: FlowingCode/ExtendedLoginAddon
Length of output: 56
🏁 Script executed:
# Check if vaadin-maven-plugin handles source-condition
rg -A 10 'vaadin-maven-plugin' pom.xml | head -50Repository: FlowingCode/ExtendedLoginAddon
Length of output: 1027
🏁 Script executed:
rg 'DemoSource|condition' pom.xml src/test/java/com/flowingcode/vaadin/addons/extendedlogin/ | head -30Repository: FlowingCode/ExtendedLoginAddon
Length of output: 2145
🏁 Script executed:
# Look for any documentation or build notes about source-condition
fd -t f -e md -e txt | xargs rg -l 'source-condition|//#if' 2>/dev/null || echo "No docs found"Repository: FlowingCode/ExtendedLoginAddon
Length of output: 87
🏁 Script executed:
# Check what Vaadin version this is and search web docs for source-condition handling
rg 'vaadin.version|project.version' pom.xml | grep -E '(vaadin|project)\.version' | head -5Repository: FlowingCode/ExtendedLoginAddon
Length of output: 368
🏁 Script executed:
# Check for FreeMarker or other templating
rg 'freemarker|FreeMarker|<#|#<|template' pom.xmlRepository: FlowingCode/ExtendedLoginAddon
Length of output: 56
🏁 Script executed:
# Look for any Maven plugin that might process these directives
rg -E 'plugin>|artifactId' pom.xml | grep -A 1 -B 1 'artifactId' | head -50Repository: FlowingCode/ExtendedLoginAddon
Length of output: 129
🏁 Script executed:
# Check if there's documentation in README or similar
fd -t f \( -name 'README*' -o -name 'CONTRIBUTING*' -o -name '.md' \) -exec rg -l 'vaadin|condition|version|build' {} \;Repository: FlowingCode/ExtendedLoginAddon
Length of output: 304
🏁 Script executed:
# Look for the DemoSource class definition
rg 'class DemoSource|interface DemoSource'Repository: FlowingCode/ExtendedLoginAddon
Length of output: 56
🏁 Script executed:
# Check git dependencies to understand what processes these directives
rg 'flowingcode.*demo|demo.*flowingcode' pom.xmlRepository: FlowingCode/ExtendedLoginAddon
Length of output: 342
🏁 Script executed:
# Look at README to understand the project structure
fd README.md -x cat {}Repository: FlowingCode/ExtendedLoginAddon
Length of output: 6195
Both @CssImport annotations are active at runtime; no build-time source preprocessing is configured.
The //#if vaadin eq directives are plain Java comments—they are not processed by any Maven plugin during the build. No preprocessing tool (maven-replacer-plugin, maven-antrun-plugin, or similar) is configured in pom.xml. As a result, both @CssImport("./styles/extended-login-styles.css") (line 42) and @CssImport("./styles/extended-login-styles-v25.css") (line 44) are compiled into the class and both stylesheets load at runtime, defeating the version-specific intent.
The @DemoSource annotation with condition attributes provides version-aware display in demo frameworks, but does not affect the compiled Java code itself.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestLoginLayoutView.java`
around lines 41 - 45, The two `@CssImport` annotations in TestLoginLayoutView (the
lines with `@CssImport`("./styles/extended-login-styles.css") and
`@CssImport`("./styles/extended-login-styles-v25.css")) are both compiled because
the //#if comments are not a preprocessing step; remove both annotations and
instead load the appropriate stylesheet at runtime in the view's constructor (or
an init method) by checking the Vaadin version or a config flag and calling
UI.getCurrent().getPage().addStyleSheet(...) or
UI.getCurrent().getElement().executeJs(...) to include the correct CSS file;
this ensures only the version-specific stylesheet is loaded instead of relying
on compile-time comments.
| private void onAccept() { | ||
| String password = this.password.getValue(); | ||
| String confirmPassword = this.confirmPassword.getValue(); | ||
| if (password != null && password.equals(confirmPassword)) { | ||
| Notification.show("Password changed."); | ||
| } else { | ||
| Notification.show("Passwords do not match."); | ||
| } |
There was a problem hiding this comment.
Reject blank passwords before showing success.
Line 79 currently accepts two empty values because they are equal, so clicking Accept can show “Password changed.” without any actual password.
Proposed fix
private void onAccept() {
String password = this.password.getValue();
String confirmPassword = this.confirmPassword.getValue();
- if (password != null && password.equals(confirmPassword)) {
+ if (password != null && !password.isBlank() && password.equals(confirmPassword)) {
Notification.show("Password changed.");
} else {
Notification.show("Passwords do not match.");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void onAccept() { | |
| String password = this.password.getValue(); | |
| String confirmPassword = this.confirmPassword.getValue(); | |
| if (password != null && password.equals(confirmPassword)) { | |
| Notification.show("Password changed."); | |
| } else { | |
| Notification.show("Passwords do not match."); | |
| } | |
| private void onAccept() { | |
| String password = this.password.getValue(); | |
| String confirmPassword = this.confirmPassword.getValue(); | |
| if (password != null && !password.isBlank() && password.equals(confirmPassword)) { | |
| Notification.show("Password changed."); | |
| } else { | |
| Notification.show("Passwords do not match."); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/flowingcode/vaadin/addons/extendedlogin/TestLoginLayoutView.java`
around lines 76 - 83, The onAccept() handler currently treats two empty strings
as a valid password change; update onAccept() to reject blank or whitespace-only
passwords before showing success by ensuring password and confirmPassword are
non-null and not blank (e.g., check password != null &&
!password.trim().isEmpty() and same for confirmPassword) and only then compare
equality (password.equals(confirmPassword)) to show "Password changed."
otherwise show the mismatch/empty error; refer to the onAccept(), password and
confirmPassword variables for locating the change.
| /*- | ||
| * #%L | ||
| * Extended Login Add-on | ||
| * %% | ||
| * Copyright (C) 2023 - 2026 Flowing Code | ||
| * %% | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * #L% | ||
| */ | ||
| /*Demo styles*/ |
There was a problem hiding this comment.
Fix the new CSS comments or whitelist the generated header.
Lines 1 and 20 currently violate Stylelint’s comment-whitespace-inside, so this file will keep the lint job red until those comment forms are normalized or the license-header pattern is explicitly excluded from that rule.
🧰 Tools
🪛 Stylelint (17.4.0)
[error] 1-1: Expected whitespace after "/*" (comment-whitespace-inside)
(comment-whitespace-inside)
[error] 20-20: Expected whitespace after "/*" (comment-whitespace-inside)
(comment-whitespace-inside)
[error] 20-20: Expected whitespace before "*/" (comment-whitespace-inside)
(comment-whitespace-inside)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/resources/META-INF/frontend/styles/extended-login-styles-v25.css`
around lines 1 - 20, The CSS license header starts with the unusual "/*-" and
lacks the required whitespace, triggering Stylelint's comment-whitespace-inside
rule for the file extended-login-styles-v25.css; fix by either (A) normalizing
the header comments in that file: change the opening "/*-" to a standard "/*"
and ensure there is a space immediately after "/*" and immediately before the
closing "*/" (so the license block and the "/*Demo styles*/" comment comply), or
(B) add an override in your stylelint config to whitelist this generated license
header by disabling or ignoring comment-whitespace-inside for that file (use an
override for the filename pattern and set the rule to null or exempt the header
pattern).
2 participants
This PR adds Vaadin 25 compatibility while maintaining support for Vaadin 24, along with several refactorings and demo improvements.
Changes:
Vaadin 24 & 25 compatibility
Code improvements
Demo improvements
Summary by CodeRabbit
Release Notes
New Features
Improvements