Skip to content

feat: add support for dynamic theme switching#140

Merged
paodb merged 1 commit intomasterfrom
theme-switch
Feb 3, 2026
Merged

feat: add support for dynamic theme switching#140
paodb merged 1 commit intomasterfrom
theme-switch

Conversation

@javier-godoy
Copy link
Member

@javier-godoy javier-godoy commented Feb 2, 2026

Summary by CodeRabbit

  • New Features

    • Dynamic theme switching with three theme options (including smooth transitions and hover-based preloading).
    • Theme selector added to the demo footer for easy access.
  • Style

    • Footer alignment improved; select control visuals adjusted.
    • Fix to select overlay to prevent stale closing state when switching themes.
  • Tests

    • Demo app initializes a default theme at startup.

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

Walkthrough

Adds a new public enum DynamicTheme implementing runtime theme selection (LUMO, AURA, BASE) with initialization, prepare, and apply lifecycle methods; integrates a theme selector into TabbedDemo; adjusts footer/select styles; and updates AppShell configuration to initialize the dynamic theme when supported.

Changes

Cohort / File(s) Summary
Core Theme Management
src/main/java/com/flowingcode/vaadin/addons/demo/DynamicTheme.java
New public enum defining LUMO, AURA, BASE with href and bgColor, Lombok getters, feature/version checks (isFeatureSupported, isFeatureInitialized, getCurrent), and lifecycle APIs: initialize(AppShellSettings), prepare(Component), apply(HasElement). Implements stylesheet injection, mouseover preloading, session-backed current theme, and client-side link toggling (with view transition fallback).
UI Integration
src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java
Added @CssImport for vaadin-select overlay and a guarded Select<DynamicTheme> theme selector in the footer wired to call apply when the feature is initialized.
Styling Updates
src/main/resources/META-INF/resources/frontend/styles/commons-demo/shared-styles.css, src/main/resources/META-INF/resources/frontend/styles/commons-demo/vaadin-select-overlay.css
Centered items in .demo-footer, adjusted vaadin-select-value-button mask/padding; added rule to disable vaadin-select-overlay animation when switching themes to avoid a stuck closing state.
AppShell / Tests
src/test/java/com/flowingcode/vaadin/addons/demo/AppShellConfiguratorImpl.java
Removed @Theme usage and added configurePage(AppShellSettings) to initialize DynamicTheme.LUMO when DynamicTheme.isFeatureSupported() is true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • mlopezFC
  • paodb
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a new DynamicTheme enum with public methods to support runtime theme switching between LUMO, AURA, and BASE themes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch theme-switch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/main/java/com/flowingcode/vaadin/addons/demo/DynamicTheme.java`:
- Around line 70-73: The calls to VaadinSession.getCurrent() in
isFeatureInitialized(), getCurrent(), and initialize() can be null and cause
NPEs; update each method to first retrieve VaadinSession session =
VaadinSession.getCurrent() and guard against null (e.g., return false from
isFeatureInitialized(), return null or throw a clear IllegalStateException from
getCurrent(), and no-op or delay initialization in initialize()) before
accessing session.getAttribute(...) or session.setAttribute(...), preserving
existing behavior when session is present and adding a clear, defensive early
return when it's absent.
- Around line 166-178: The injected JS in component.getElement().executeJs calls
document.startViewTransition and then accesses properties on link without
verifying link exists, which can throw in non-Chromium browsers or when the
stylesheet isn't present; update the script to first check that
document.startViewTransition is a function and fall back to executing the
transition callback directly if unavailable, and add a null check for link
(e.g., if (link) { ... }) before reading or writing link.rel or link.disabled
when iterating over the href list (note the passed href parameter $0), so the
code safely handles missing <link> elements and browsers without
startViewTransition support.
🧹 Nitpick comments (2)
src/main/java/com/flowingcode/vaadin/addons/demo/DynamicTheme.java (1)

107-116: Consider using theme.getHref() instead of hardcoded paths.

The enum already defines href for each theme. Using theme.getHref() would be more consistent and DRY.

♻️ Proposed refactor
-    switch (theme) {
-      case AURA:
-        settings.addLink(Position.APPEND, "stylesheet", "aura/aura.css");
-        break;
-      case LUMO:
-        settings.addLink(Position.APPEND, "stylesheet", "lumo/lumo.css");
-        break;
-      default:
-        break;
+    if (theme.getHref() != null) {
+      settings.addLink(Position.APPEND, "stylesheet", theme.getHref());
     }
src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java (1)

132-142: Consider null-safety for value change handler.

While Select typically won't fire change events with null values when properly configured, adding a defensive check prevents potential NPEs.

🛡️ Proposed defensive check
-      themeSelect.addValueChangeListener(ev -> ev.getValue().apply(this));
+      themeSelect.addValueChangeListener(ev -> {
+        DynamicTheme theme = ev.getValue();
+        if (theme != null) {
+          theme.apply(this);
+        }
+      });

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2026

@javier-godoy javier-godoy requested a review from mlopezFC February 2, 2026 20:00
@javier-godoy javier-godoy marked this pull request as ready for review February 2, 2026 20:00
@paodb paodb merged commit 156e423 into master Feb 3, 2026
3 of 5 checks passed
@github-project-automation github-project-automation bot moved this from To Do to Pending release in Flowing Code Addons Feb 3, 2026
@paodb paodb deleted the theme-switch branch February 3, 2026 18:29
@paodb paodb moved this from Pending release to Done in Flowing Code Addons Feb 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants