nuvoleweb / ui_patterns

[NOTE] Development has moved to https://drupal.org/project/ui_patterns
https://drupal.org/project/ui_patterns
GNU General Public License v2.0
85 stars 56 forks source link

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

Closed msnassar closed 1 year ago

msnassar commented 4 years ago

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!

vever001 commented 4 years ago

+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.

donquixote commented 3 years ago

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 commented 3 years ago

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?

kp77 commented 3 years ago

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

mika2na commented 1 year ago

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