Skip to content

Commit

Permalink
feat!: Theme annotation should be only on AppShellConfigurator (#9313)
Browse files Browse the repository at this point in the history
Having the `@Theme` annotation on Flow views or router layouts will not be allowed anymore, it should be on `AppShellConfigurator` instead.

Fixes #9092
  • Loading branch information
caalador authored Nov 9, 2020
1 parent 73ebb9e commit c5f5319
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import javax.servlet.ServletRegistration;
import javax.servlet.annotation.HandlesTypes;
import javax.servlet.annotation.WebListener;

import java.io.Serializable;
import java.lang.annotation.Annotation;
import java.util.ArrayList;
Expand Down Expand Up @@ -51,6 +50,8 @@
import com.vaadin.flow.server.VaadinServlet;
import com.vaadin.flow.server.VaadinServletContext;
import com.vaadin.flow.server.startup.ServletDeployer.StubServletConfig;
import com.vaadin.flow.theme.NoTheme;
import com.vaadin.flow.theme.Theme;

import static com.vaadin.flow.server.AppShellRegistry.ERROR_HEADER_NO_APP_CONFIGURATOR;
import static com.vaadin.flow.server.AppShellRegistry.ERROR_HEADER_NO_SHELL;
Expand All @@ -64,7 +65,8 @@
*/
@HandlesTypes({ AppShellConfigurator.class, Meta.class, Meta.Container.class,
PWA.class, Inline.class, Inline.Container.class, Viewport.class,
BodySize.class, PageTitle.class, PageConfigurator.class, Push.class })
BodySize.class, PageTitle.class, PageConfigurator.class, Push.class,
Theme.class, NoTheme.class })
// @WebListener is needed so that servlet containers know that they have to run
// it
@WebListener
Expand Down
4 changes: 2 additions & 2 deletions flow-server/src/main/java/com/vaadin/flow/theme/NoTheme.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
* Flow uses the following logic to determine which theme to use for the
* application:
* <ul>
* <li>If a {@link Theme} annotation is found at the root navigation level, the
* <li>If a {@link Theme} annotation is found on the AppShellConfigurator, the
* theme defined by it is used.
* <li>If a {@link NoTheme} annotation is found at the root navigation level,
* <li>If a {@link NoTheme} annotation is found on the AppShellConfigurator,
* theming is disabled.
* <li>If the <code>com.vaadin.flow.theme.lumo.Lumo</code> class is available in
* the classpath (which comes from the vaadin-lumo-theme project), then it is
Expand Down
37 changes: 7 additions & 30 deletions flow-server/src/main/java/com/vaadin/flow/theme/Theme.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import com.vaadin.flow.router.Route;
import com.vaadin.flow.router.RouterLayout;

/**
* Defines that there is a theme to use and defines the theme handler
* implementation.
Expand All @@ -36,47 +33,27 @@
* By default {@code com.vaadin.flow.theme.lumo.Lumo} theme is used if it's in
* the classpath. You may disable theming with {@link NoTheme} annotation.
* <p>
* {@link Theme} annotation should be added to your root navigation level,
* {@link RouterLayout} or to the top level @{@link Route}.
* {@link Theme} annotation should be added to the AppShellConfigurator
* implementation.
*
* <p>
* Defining different Themes for different views will end throwing an exception.
* Only a single theme can be defined and having multiple instances will throw
* an exception.
*
* <p>
* Here are examples:
*
* <ul>
* <li>On the navigation root
*
* <pre>
* <code>
* &#64;Route(value = "")
* &#64;Theme(Lumo.class)
* public class Main extends Div {
* }
* </code>
* </pre>
*
* <li>on the top level router layout
* Here is an example:
*
* <pre>
* <code>
* &#64;Theme(MyTheme.class)
* public class MainLayout extends Div implements RouterLayout {
* }
*
* &#64;Route(value = "editor", layout = MainLayout.class)
* public class Editor extends Div {
* &#64;Theme(Lumo.class)
* public class MyAppShell implements AppShellConfigurator {
* }
* </code>
* </pre>
*
* </ul>
*
* @see AbstractTheme
* @see NoTheme
* @see RouterLayout
* @see Route
*
* @author Vaadin Ltd
* @since 1.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
import com.vaadin.flow.server.VaadinServletRequest;
import com.vaadin.flow.shared.communication.PushMode;
import com.vaadin.flow.shared.ui.Transport;
import com.vaadin.flow.theme.AbstractTheme;
import com.vaadin.flow.theme.Theme;

import static com.vaadin.flow.server.DevModeHandler.getDevModeHandler;
import static org.hamcrest.CoreMatchers.containsString;
Expand All @@ -78,6 +80,7 @@ public static class MyAppShellWithoutAnnotations
@BodySize(height = "my-height", width = "my-width")
@PageTitle("my-title")
@Push(value = PushMode.MANUAL, transport = Transport.WEBSOCKET)
@Theme(AbstractTheme.class)
public static class MyAppShellWithMultipleAnnotations
implements AppShellConfigurator {
}
Expand All @@ -91,6 +94,7 @@ public static class MyAppShellWithMultipleAnnotations
@BodySize(height = "my-height", width = "my-width")
@PageTitle("my-title")
@Push(value = PushMode.MANUAL, transport = Transport.WEBSOCKET)
@Theme(AbstractTheme.class)
public static class OffendingClass {
}

Expand Down Expand Up @@ -370,7 +374,7 @@ public void should_throw_when_offendingClass() throws Exception {
exception.expectMessage(containsString(
"Found app shell configuration annotations in non"));
exception.expectMessage(containsString(
"- @Meta, @Inline, @Viewport, @BodySize, @Push" + " from"));
"- @Meta, @Inline, @Viewport, @BodySize, @Push, @Theme" + " from"));
classes.add(MyAppShellWithoutAnnotations.class);
classes.add(OffendingClass.class);
initializer.process(classes, servletContext);
Expand Down

0 comments on commit c5f5319

Please sign in to comment.