ui_patterns icon indicating copy to clipboard operation
ui_patterns copied to clipboard

Unable to detect a pattern from default theme having same id as the pattern in the base theme

Open msnassar opened this issue 4 years ago • 4 comments

Steps to reproduce: Create a pattern in your base theme. Copy the previously created pattern to your sub theme (default theme). ui patterns library detects the pattern defined in your base theme instead of the one in the default theme.

I think a pattern defined in the default theme (having same id as a pattern in the base theme) should be the one detected. As it overrides the pattern in the base theme!

msnassar avatar Sep 03 '20 12:09 msnassar

+1 on this!

By looking at the code, it looks like this behavior was done on purpose. Not sure if that's the case or why this would be desirable. So unless there is a strong reason not to, IMO the sub-themes should be able to easily override any pattern from the base theme. In other words, respect the usual flow of overrides: modules, base themes, sub-themes.

And the logic could be simplified to:

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

Same logic as \Drupal\Core\Layout\LayoutPluginManager and \Drupal\breakpoint\BreakpointManager.

vever001 avatar Sep 04 '20 14:09 vever001

I made an interesting observation: Without the patch, the yml definition from the base theme is used, but the template from the sub-theme. The reason is that each pattern is also a theme hook, and the sub-theme template overrides the base theme template via the theme system.

With the patch, both the yml definition and the template from the sub-theme are used.

After removing the yml in the sub-theme, the yml from the base theme and the template from the sub-theme are used - which is good, I assume.

donquixote avatar Oct 05 '20 16:10 donquixote

Module-provided patterns vs theme-provided patterns

The solution proposed here fixes the order of base theme vs sub-theme. However, what about module-provided patterns? Should they be overridden by theme-provided patterns? For the template this should already work through the theme layer, but for the yml definition it won't, because module directories are added last.

Pattern inheritance compatibility validation?

Also, I wonder if there should be any validation to check the compatibility of the theme-provided pattern vs the module-provided pattern. Do we need something like LSP (Liskov substitution principle) for patterns?

donquixote avatar Oct 05 '20 16:10 donquixote

I'd like to add to this problem that I also had errors during site install with existing config, when using patterns in layout builder, because during install the default theme is stark.

@vever001 's suggestion would also fix that problem, so I created a follow up PR that uses that method: #318

kp77 avatar Dec 09 '20 12:12 kp77

Moved to Drupal.org https://www.drupal.org/project/ui_patterns/issues/3311471

mika2na avatar Sep 23 '22 13:09 mika2na