Skip to content

feat: add getThemeAttribute and applyThemeAttribute methods AND remove!: remove deprecated methods and classes #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paodb merged 3 commits into from
Nov 19, 2025
Merged

feat: add getThemeAttribute and applyThemeAttribute methods AND remove!: remove deprecated methods and classes #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17cfe7f
build: set version to 5.0.0-SNAPSHOT
javier-godoy Nov 18, 2025
9189664
feat: add getThemeAttribute and applyThemeAttribute methods
javier-godoy Nov 18, 2025
279e5b6
remove!: remove deprecated methods and classes
javier-godoy Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>com.flowingcode.vaadin.addons.demo</groupId>
<artifactId>commons-demo</artifactId>
<version>4.4.0-SNAPSHOT</version>
<version>5.0.0-SNAPSHOT</version>

<name>Commons Demo</name>
<description>Common classes for add-ons demo</description>
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/flowingcode/vaadin/addons/demo/DefaultSourceUrlResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public Optional<String> resolveURL(TabbedDemo demo, Class<?> annotatedClass,
DemoSource annotation) {
String demoFile;
String url = annotation.value();
if (url.equals(DemoSource.GITHUB_SOURCE) || url.equals(DemoSource.DEFAULT_VALUE)) {
if (url.equals(DemoSource.DEFAULT_VALUE)) {
String className;
if (annotation.clazz() == DemoSource.class) {
className = annotatedClass.getName().replace('.', '/');
Expand Down
6 changes: 1 addition & 5 deletions src/main/java/com/flowingcode/vaadin/addons/demo/DemoSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* #%L
* Commons Demo
* %%
* Copyright (C) 2020 - 2024 Flowing Code
* Copyright (C) 2020 - 2025 Flowing Code
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -46,10 +46,6 @@
@Target(ElementType.TYPE)
public @interface DemoSource {

/** @deprecated. Use {@link #DEFAULT_VALUE} */
@Deprecated
static final String GITHUB_SOURCE = "__GITHUB__";

static final String DEFAULT_VALUE = "__DEFAULT__";

/**
Expand Down
98 changes: 0 additions & 98 deletions src/main/java/com/flowingcode/vaadin/addons/demo/RouteTabs.java
View file
Open in desktop

This file was deleted.

49 changes: 7 additions & 42 deletions src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* #%L
* Commons Demo
* %%
* Copyright (C) 2020 - 2024 Flowing Code
* Copyright (C) 2020 - 2025 Flowing Code
* %%
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -93,7 +93,9 @@ public TabbedDemo() {
themeCB.setValue(false);
themeCB.addClassName("smallcheckbox");
themeCB.addValueChangeListener(cb -> {
applyTheme(getElement(), themeCB.getValue());
boolean useDarkTheme = themeCB.getValue();
String theme = useDarkTheme ? "dark" : "";
applyThemeAttribute(getElement(), theme);
});
Comment on lines 93 to 99
Copy link

@coderabbitai coderabbitai bot Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Theme toggle handler correctly delegates to the new theme API

Using the checkbox value to compute "dark" vs "" and calling applyThemeAttribute(getElement(), theme) keeps the UI behavior clear and ties directly into the new session-backed theme attribute.

Minor suggestion: you can use the event’s value (cb.getValue()) instead of closing over themeCB, but the current implementation is perfectly fine.

If you want to double-check expected Vaadin theming behavior for the "dark" theme attribute, please confirm against the Vaadin 24/25 theming docs.

🌐 Web query:

Vaadin Flow theming: how is the `"theme"` attribute on `documentElement` interpreted, and is an empty string equivalent to no theme?

💡 Result:

Short answer: The document's theme attribute is treated as a space‑separated token list (a ThemeList) and is used with attribute selectors like [theme~="dark"]. An empty string is not the same as the attribute being absent — Flow distinguishes "attribute not set" (getAttribute() -> null) from "attribute set but no tokens" (getThemeName() -> empty string). Use the ThemeList / HasTheme API to add/remove tokens rather than manipulating raw strings. [1][2][3]

Sources:

  • HasTheme API: getThemeName/getThemeNames behavior (empty string vs null). [1]
  • Element API: attributes and getThemeList / attribute semantics (empty value rendering, getAttribute() -> null if not set). [2]
  • Vaadin blog on dynamic theming and use of [theme~="..."] selectors. [3]

I'll verify the implementation of applyThemeAttribute to confirm whether the current code correctly handles the theme attribute behavior described in the web search results.

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# Find applyThemeAttribute method definition
rg -nP 'applyThemeAttribute' --type=java -B2 -A10

Length of output: 4163

🏁 Script executed:

#!/bin/bash
# Search for any theme attribute handling or Element.setAttribute calls in the codebase
rg -nP 'setAttribute.*theme|getTheme|setTheme' --type=java -C3

Length of output: 2688

Critical bug: setting theme attribute to empty string instead of removing it

The applyThemeAttribute method (lines 324–334) always calls setAttribute('theme', $0), which means when theme="" is passed (line 97), the DOM receives setAttribute('theme', ""). Per Vaadin theming semantics, an empty string is not the same as the attribute being absent — Flow distinguishes "attribute not set" (getAttribute() → null) from "attribute set but no tokens" (getThemeName() → empty string).

Fix: Update applyThemeAttribute to call removeAttribute('theme') when the theme parameter is empty:

public static void applyThemeAttribute(Element element, String theme) {
  VaadinSession.getCurrent().setAttribute(THEME_NAME, theme);

  String script;
  if (theme.isEmpty()) {
    if (element.getTag().equalsIgnoreCase("iframe")) {
      script = "this.contentWindow.document.documentElement.removeAttribute('theme')";
    } else {
      script = "document.documentElement.removeAttribute('theme');";
    }
  } else {
    if (element.getTag().equalsIgnoreCase("iframe")) {
      script = "this.contentWindow.document.documentElement.setAttribute('theme', $0)";
    } else {
      script = "document.documentElement.setAttribute('theme', $0);";
    }
  }

  if (theme.isEmpty()) {
    element.executeJs(script);
  } else {
    element.executeJs(script, theme);
  }
}
🤖 Prompt for AI Agents 
In src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java around lines
93–99 (and update applyThemeAttribute implementation at lines ~324–334), the
current logic calls setAttribute('theme', "") when theme is empty which leaves
an empty attribute instead of removing it; change applyThemeAttribute so it
stores the theme in VaadinSession as before but builds two branches: if theme is
empty, generate JS that calls removeAttribute('theme') (use
this.contentWindow.document.documentElement for iframe tags, otherwise
document.documentElement), and call element.executeJs(script) with no
parameters; if theme is non-empty, generate JS that calls setAttribute('theme',
$0) (same iframe vs document handling) and call element.executeJs(script,
theme).

Copy link
Member Author

@javier-godoy javier-godoy Nov 18, 2025

Choose a reason for hiding this comment

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

@coderabbitai Create an issue about it. The scope of this PR is only removing the deprecated methods, not fixing behavior that was already there.

CommonsDemo/src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java

Lines 353 to 357 in 9e9f1e8

@Deprecated
public static void applyTheme(Element element, boolean useDarkTheme) {
String theme = useDarkTheme ? "dark" : "";
applyTheme(element, theme);
}

Copy link

@coderabbitai coderabbitai bot Nov 18, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for the theme attribute handling bug

gh issue create \
  --repo FlowingCode/CommonsDemo \
  --title "Theme attribute handling: empty string should remove attribute instead of setting it to empty" \
  --assignee javier-godoy \
  --body "## Description

The \`applyThemeAttribute\` method in \`TabbedDemo.java\` currently calls \`setAttribute('theme', '')\` when the theme parameter is an empty string. According to Vaadin theming semantics, an empty string attribute is not the same as the attribute being absent — Flow distinguishes \"attribute not set\" (getAttribute() → null) from \"attribute set but no tokens\" (getThemeName() → empty string).

## Current Behavior

When the theme is toggled off (dark mode unchecked), the code sets:
\`\`\`javascript
document.documentElement.setAttribute('theme', '')
\`\`\`

This leaves an empty \`theme\` attribute on the document element.

## Expected Behavior

When the theme parameter is empty, the attribute should be removed entirely:
\`\`\`javascript
document.documentElement.removeAttribute('theme')
\`\`\`

## Suggested Fix

Update \`applyThemeAttribute\` to check if the theme is empty and call \`removeAttribute('theme')\` instead of \`setAttribute('theme', '')\`.

## Context

- Flagged during review of PR #127: https://github.com/FlowingCode/CommonsDemo/pull/127#discussion_r2539569353
- Requested by: @javier-godoy
- This behavior existed in the previous implementation and is out of scope for PR #127, which focuses on removing deprecated methods"

Length of output: 53

@javier-godoy I've created issue #128 to track the theme attribute handling bug: #128

I've assigned it to you and included technical details along with references to this PR and comment thread. You're absolutely right to keep PR #127 focused on removing the deprecated methods—the theme attribute behavior can be addressed separately. 👍

🧬 Code Graph Analysis Results

[src/test/java/com/flowingcode/vaadin/addons/demo/AdHocDemo.java, lines 27-36]

@Route(value = "demo/demo-with-adhoc-source", layout = Demo.class)
@PageTitle("Demo with source in a different file")
@DemoSource("/src/test/java/com/flowingcode/vaadin/addons/demo/AdHocSource.java")
public class AdHocDemo extends Div {

  public AdHocDemo() {
    add(new Span("This span is added to the demo view"));
  }

}

[src/main/java/com/flowingcode/vaadin/addons/demo/SplitLayoutDemo.java, lines 32-114]

@SuppressWarnings("serial")
class SplitLayoutDemo extends Composite<SplitLayout> {

  private MultiSourceCodeViewer code;
  private Component demo;
  private SourcePosition sourcePosition;

  public SplitLayoutDemo(Component demo, String sourceUrl, SourcePosition sourcePosition) {
    this(demo, Arrays.asList(new SourceCodeTab(sourceUrl, sourcePosition)));
  }

  public SplitLayoutDemo(Component demo, List<SourceCodeTab> tabs) {
    getContent().setOrientation(Orientation.HORIZONTAL);

    Map<String, String> properties = new HashMap<>();
    properties.put("vaadin", VaadinVersion.getVaadinVersion());
    properties.put("flow", Version.getFullVersion());

    code = new MultiSourceCodeViewer(tabs, properties);
    this.demo = demo;
    setSourcePosition(code.getSourcePosition());

    getContent().setSizeFull();
  }

  private void setSourcePosition(SourcePosition position) {
    if (!position.equals(sourcePosition)) {
      getContent().removeAll();
      switch (position) {
        case PRIMARY:
          getContent().addToPrimary(code);
          getContent().addToSecondary(demo);
          break;
        case SECONDARY:
        default:
          getContent().addToPrimary(demo);
          getContent().addToSecondary(code);
      }
      sourcePosition = position;
    }
  }

  public void toggleSourcePosition() {
    setSourcePosition(sourcePosition.toggle());
  }

  public void setOrientation(Orientation o) {
    getContent().setOrientation(o);
    getContent()
        .getPrimaryComponent()
        .getElement()
        .setAttribute("style", "width: 100%; height: 100%");
    code.setSizeFull();
  }

  public Orientation getOrientation() {
    return getContent().getOrientation();
  }

  public void setSplitterPosition(int pos) {
    getContent().setSplitterPosition(pos);
  }

  public void setSizeFull() {
    getContent().setSizeFull();
  }

  public void showSourceCode() {
    getContent().setSplitterPosition(50);
  }

  public void hideSourceCode() {
    switch (sourcePosition) {
      case PRIMARY:
        getContent().setSplitterPosition(0);
        break;
      case SECONDARY:
        getContent().setSplitterPosition(100);
        break;
    }
  }

}

[src/test/java/com/flowingcode/vaadin/addons/demo/MultiSourceDemo.java, lines 27-51]

@Route(value = "demo/multisource", layout = Demo.class)
@PageTitle("Demo with multiple sources")
// show-source @DemoSource
// show-source @DemoSource(clazz = AdditionalSources.class)
// show-source @DemoSource("/src/test/resources/META-INF/resources/frontend/multi-source-demo.css")
@DemoSource
@DemoSource(clazz = AdditionalSources.class)
@DemoSource("/src/test/resources/META-INF/resources/frontend/multi-source-demo.css")
@StyleSheet("./multi-source-demo.css")
public class MultiSourceDemo extends Div {
  public MultiSourceDemo() {

    // begin-block main
    Div div = new Div("This is the main source");
    div.addClassName("custom-style");
    SourceCodeViewer.highlightOnHover(div, "main");
    add(div);
    // end-block

    Button button1 = new Button("Highlight code in AdditionalSources");
    SourceCodeViewer.highlightOnClick(button1, "AdditionalSources.java#fragment");
    add(button1);
  }

}

[src/test/java/com/flowingcode/vaadin/addons/demo/SampleDemoDefault.java, lines 27-42]

@Route(value = "demo/demo-with-source", layout = Demo.class)
@PageTitle("Demo with source")
@DemoSource
public class SampleDemoDefault extends Div {

  public SampleDemoDefault() {
    add(new Span("Demo component with defaulted @DemoSource annotation"));
    // show-source System.out.println("this line will be displayed in the code snippet");
    this.getClass(); // hide-source (this line will not be displayed in the code snippet)
    // #if vaadin ge 23
    // show-source System.out.println("conditional code for Vaadin 23+");
    // #elif vaadin ge 14
    // show-source System.out.println("conditional code for Vaadin 14-22");
    // #endif
  }
}

[src/test/java/com/flowingcode/vaadin/addons/demo/Demo.java, lines 26-40]

@Route("demo")
@RouteAlias("")
GithubLink("https://github.com/FlowingCode/CommonsDemo")
public class Demo extends TabbedDemo {

  public Demo() {
    addDemo(new LegacyDemo());
    addDemo(SampleDemo.class);
    addDemo(SampleDemoDefault.class);
    addDemo(SampleDemoHighlight.class);
    addDemo(AdHocDemo.class);
    addDemo(MultiSourceDemo.class);
    addDemo(SourcePositionDemo.class);
  }
}

[src/test/java/com/flowingcode/vaadin/addons/demo/DialogDemoHelperViewer.java, lines 30-38]

@SuppressWarnings("serial")
public class DialogDemoHelperViewer implements DemoHelperViewer {

  @Override
  public void show(Component content) {
    new Dialog(content).open();
  }

}

[src/test/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewerElement.java, lines 25-28]

@Element("code-viewer")
public class SourceCodeViewerElement extends TestBenchElement {
  
}

footer = new HorizontalLayout();
footer.setWidthFull();
Expand All @@ -118,32 +120,6 @@ public TabbedDemo() {
setSizeFull();
}

/**
* Add a tab with a {@code demo} component. The tab label and source code URL are retrieved from
* the {@link PageTitle} (required) and {@link DemoSource} (optional) annotations in the demo
* class, respectively.
*
* @param demo the demo instance
*/
@Deprecated
public void addDemo(Component demo) {
String label = Optional.ofNullable(demo.getClass().getAnnotation(PageTitle.class))
.map(PageTitle::value).orElse(demo.getClass().getSimpleName());

addDemo(demo, label, null);
}

/**
* @param demo the demo instance
* @param label the demo name (tab label)
* @param sourceCodeUrl ignored.
*/
@Deprecated
public void addDemo(Component demo, String label, String sourceCodeUrl) {
tabs.addLegacyTab(label, demo);
updateVisibility();
}

/**
* Add a tab with a demo component.
*
Expand Down Expand Up @@ -186,11 +162,6 @@ public void addDemo(Class<? extends Component> clazz) {
addDemo(clazz, label);
}

@Deprecated
public void addDemo(Component demo, String label) {
addDemo(demo, label, null);
}

@Override
public void showRouterLayoutContent(HasElement content) {
Component demo = (Component) content;
Expand Down Expand Up @@ -229,7 +200,7 @@ public void showRouterLayoutContent(HasElement content) {
updateFooterButtonsVisibility();
getElement().insertChild(1, content.getElement());

applyTheme(getElement(), getThemeName());
applyThemeAttribute(getElement(), getThemeAttribute());
}

private static SourceUrlResolver resolver = null;
Expand Down Expand Up @@ -345,18 +316,12 @@ public void setOrientation(Orientation orientation) {

private static final String THEME_NAME = TabbedDemo.class.getName() + "#THEME_NAME";

public static String getThemeName() {
public static String getThemeAttribute() {
return (String) Optional.ofNullable(VaadinSession.getCurrent().getAttribute(THEME_NAME))
.orElse("");
}

@Deprecated
public static void applyTheme(Element element, boolean useDarkTheme) {
String theme = useDarkTheme ? "dark" : "";
applyTheme(element, theme);
}

public static void applyTheme(Element element, String theme) {
public static void applyThemeAttribute(Element element, String theme) {
VaadinSession.getCurrent().setAttribute(THEME_NAME, theme);

String script;
Expand Down