Skip to content
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.

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

OSGi: make tests for Lumo theme #55

Open
denis-anisimov opened this issue Jan 21, 2019 · 0 comments
Open

OSGi: make tests for Lumo theme #55

denis-anisimov opened this issue Jan 21, 2019 · 0 comments

Comments

@denis-anisimov
Copy link

We need a test for vaadin/flow#4847.

I hadn't done this tests as a part of the fix for the ticket because we don't have any tests for built-in themes at all.
I've made IT tests for Lumo and Material themes.

I suggest to make a bundle from lumo tests in the same way as it has been done with test-root-context.

It doesn't matter how it will be done in the end but I'm pretty sure that this way requires less efforts:

  • make a new bundle from lumo views (existing Lumo test module). As mentioned: exactly in the same way as for test-root-context.
  • deploy this bundle to felix in the existing OSGi IT tests.
  • run IT (existing) test for Lumo .

There is one issue which you should be aware of: we assume that our test-root-context are executed without any theme (even if the don't have NoTheme annotation).

Deployed lumo theme bundle will activate Lumo theme by default.
This may be unexpected and may be need to be prevented.

I suggest to use a custom UI class with overridden getThemeFor.
This method should not return Lumo if there is no explicitly defined theme.
It should return theme if there is explicitly defined theme.
And there should be some marker for classes which wants to have implicit Lumo theme ( some kind of opposite for NoTheme annotation).

We need to test:

  • class (only one single dedicated class) which has no explicit Lumo annotation but gets the Lumo theme by default (can be done via checking some custom annotation added to the class).
  • class which has explicit Lumo annotation.

All other classes (most likely) should not be themed using Lumo (even though it's in the classpath and automatic theme discovering works). As I said this can be done via overriding getThemeFor.

In case of any questions about the implementation suggestion ask me.

@pleku pleku transferred this issue from vaadin/flow Mar 29, 2021
@mshabarov mshabarov moved this to 🔖 Normal Priority (P2) in Vaadin Flow bugs & maintenance (Vaadin 10+) Sep 27, 2023
@mshabarov mshabarov moved this from 🔖 Normal Priority (P2) to Internal Backlog / Technical Debt in Vaadin Flow bugs & maintenance (Vaadin 10+) Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Internal Backlog / Technical Debt
Development

No branches or pull requests

1 participant