Theme System: Fixed theme view before/after issues

- Updated the system to work with modules.
- Updated module docs to consider namespacing.
- Fixed view loading and registration event ordering.
- Fixed checking if views are registered.

Author: ssddanbrown

app/App/Providers/ThemeServiceProvider.php

@@ -35,9 +35,9 @@ public function boot(): void
         $themeService->readThemeActions();
         $themeService->dispatch(ThemeEvents::APP_BOOT, $this->app);
 
-        $themeViews = new ThemeViews();
+        $themeViews = new ThemeViews($viewFactory->getFinder());
+        $themeViews->registerViewPathsForTheme($themeService->getModules());
         $themeService->dispatch(ThemeEvents::THEME_REGISTER_VIEWS, $themeViews);
-        $themeViews->registerViewPathsForTheme($viewFactory->getFinder(), $themeService->getModules());
         if ($themeViews->hasRegisteredViews()) {
             $viewFactory->share('__themeViews', $themeViews);
             Blade::directive('include', function ($expression) {

app/Theming/ThemeViews.php

@@ -17,21 +17,26 @@ class ThemeViews
      */
     protected array $afterViews = [];
 
+    public function __construct(
+        protected FileViewFinder $finder
+    ) {
+    }
+
     /**
      * Register any extra paths for where we may expect views to be located
-     * with the provided FileViewFinder, to make custom views available for use.
+     * with the FileViewFinder, to make custom views available for use.
      * @param ThemeModule[] $modules
      */
-    public function registerViewPathsForTheme(FileViewFinder $finder, array $modules): void
+    public function registerViewPathsForTheme(array $modules): void
     {
         foreach ($modules as $module) {
             $moduleViewsPath = $module->path('views');
             if (file_exists($moduleViewsPath) && is_dir($moduleViewsPath)) {
-                $finder->prependLocation($moduleViewsPath);
+                $this->finder->prependLocation($moduleViewsPath);
             }
         }
 
-        $finder->prependLocation(theme_path());
+        $this->finder->prependLocation(theme_path());
     }
 
     /**
@@ -70,19 +75,21 @@ public function renderAfter(string $targetView, string $localView, int $priority
 
     public function hasRegisteredViews(): bool
     {
-        return !empty($this->beforeViews) && !empty($this->afterViews);
+        return !empty($this->beforeViews) || !empty($this->afterViews);
     }
 
     protected function registerAdjacentView(array &$location, string $targetView, string $localView, int $priority = 50): void
     {
-        $viewPath = theme_path($localView . '.blade.php');
-        if (!file_exists($viewPath)) {
-            throw new ThemeException("Expected registered view file at \"{$viewPath}\" does not exist");
+        try {
+            $viewPath = $this->finder->find($localView);
+        } catch (\InvalidArgumentException $exception) {
+            throw new ThemeException("Expected registered view file with name \"{$localView}\" could not be found.");
         }
 
         if (!isset($location[$targetView])) {
             $location[$targetView] = [];
         }
+
         $location[$targetView][$viewPath] = $priority;
     }
 

dev/docs/theme-system-modules.md

@@ -58,6 +58,9 @@ Here are some general best practices when it comes to creating modules:
 - Use a unique name and clear description so the user can understand the purpose of the module.
 - Increment the metadata version on change, keeping to [semver](https://semver.org/) to indicate compatibility of new versions.
 - Where possible, prefer to [insert views before/after](logical-theme-system.md#custom-view-registration-example) instead of overriding existing views, to reduce likelihood of conflicts or update troubles.
+- When using/registering custom views, use some level of unique namespacing within the view path to prevent potential conflicts with other customizations.
+  - For example, I may store a view within my module as `views/my-module-name-welcome.blade.php`, to be registered as 'my-module-name-welcome'.
+  - This is important since views may be resolved from other modules or the active theme, which may/will override your module level view.
 
 ### Distribution Format
 

tests/Theme/ThemeModuleTests.php tests/Theme/ThemeModuleTest.phptests/Theme/ThemeModuleTests.php renamed to tests/Theme/ThemeModuleTest.php

@@ -5,7 +5,7 @@
 use BookStack\Facades\Theme;
 use Tests\TestCase;
 
-class ThemeModuleTests extends TestCase
+class ThemeModuleTest extends TestCase
 {
     public function test_modules_loaded_on_theme_load()
     {
@@ -207,6 +207,19 @@ public function test_logical_functions_file_loaded_from_module_and_it_runs_along
         });
     }
 
+    public function test_module_can_use_theme_view_render_functions()
+    {
+        $this->usingModuleFolder(function (string $moduleFolderPath) {
+            file_put_contents($moduleFolderPath . '/functions.php', "<?php\n\BookStack\Facades\Theme::listen(\BookStack\Theming\ThemeEvents::THEME_REGISTER_VIEWS, fn(\$views) => \$views->renderBefore('layouts.parts.header', 'cat', 100));");
+            mkdir($moduleFolderPath . '/views', 0777, true);
+            file_put_contents($moduleFolderPath . '/views/cat.blade.php', 'mysupercatispouncy');
+
+            $this->refreshApplication();
+
+            $this->asAdmin()->get('/')->assertSee('mysupercatispouncy');
+        });
+    }
+
     protected function usingModuleFolder(callable $callback): void
     {
         $this->usingThemeFolder(function (string $themeFolder) use ($callback) {