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

Support Multi-theme Sites #310

Conversation

WidgetsBurritos
Copy link

Per issue in #308, this PR attempts to resolve the case where multiple themes are used on a site.

I created a separate test-only PR in #309 to demonstrate the issue.

Crossing my fingers that the tests pass here as I couldn't get the tests to run inside of my docker container due to some chrome issues, but really I'm just reusing existing tests, but starting off with a different default theme, and switching to the other theme after the fact.

@WidgetsBurritos WidgetsBurritos force-pushed the 308-support-multitheme-sites branch from 1d0af61 to 37e584f Compare September 25, 2020 04:54
@WidgetsBurritos
Copy link
Author

I ended up killing the test for now. I got docker working locally but there's something strange going on I need to dig into. I also swapped the order of the path declarations:

   protected function getDirectories() {
     return $this->themeHandler->getThemeDirectories() + $this->moduleHandler->getModuleDirectories();
   }

Instead of

   protected function getDirectories() {
     return $this->moduleHandler->getModuleDirectories() + $this->themeHandler->getThemeDirectories();
   }

This seems to prevent the failure on the UiPatternsLibraryOverviewTest we got in the first run. I want to confirm everything works here now, and will sort out the theme switching test in a bit.

@WidgetsBurritos WidgetsBurritos marked this pull request as draft September 25, 2020 16:40
@kp77
Copy link

kp77 commented Dec 8, 2020

Hi @WidgetsBurritos,

I debugged the failing test in UiPatternsLibraryOverviewTest with the original order for the union of the path arrays:

   protected function getDirectories() {
     return $this->moduleHandler->getModuleDirectories() + $this->themeHandler->getThemeDirectories();
   }

I found that the issue is that the order of the displayed patterns in the overview test is different when using the module paths + theme paths version. This can be fixed by moving the 3 theme patterns (simple, with_variants and with_custom_theme_hook) to the bottom of the file in modules/ui_patterns_library/tests/fixtures/overview-page-patterns.yml

By making this change in the fixture I could run all the tests without failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants