Fix theme switcher

[jcastroa87]

This PR is to fix Theme Switcher on Demo, issue: https://github.com/Laravel-Backpack/demo/issues/524

[pxpm]

Hey @jorgetwgroup I think we are on the right track here like we discussed, but I think the solution is doing a little bit more than what's needed.

From my understanding, what we are doing here is the same as going to all theme service providers, and remove the check if the theme is active in loadViews() right ?

public function loadViews()
    {
-       // if this addon is a theme, but isn't active, don't load any views
-        if ($this->theme && ! $this->packageIsActiveTheme()) {
-           return;
-        }

        // Load published views
        if (is_dir($this->publishedViewsPath())) {
            $this->loadViewsFrom($this->publishedViewsPath(), $this->vendorNameDotPackageName());
        }

        // Fallback to package views
        $this->loadViewsFrom($this->packageViewsPath(), $this->vendorNameDotPackageName());

        // Add default ViewNamespaces
        foreach (['buttons', 'columns', 'fields', 'filters', 'widgets'] as $viewNamespace) {
            if ($this->packageDirectoryExistsAndIsNotEmpty("resources/views/$viewNamespace")) {
                ViewNamespaces::addFor($viewNamespace, $this->vendorNameDotPackageName()."::{$viewNamespace}");
            }
        }

        // Add basset view path
        Basset::addViewPath($this->packageViewsPath());
    }

I've just tried out doing what I said (removing the check from loadViews() in theme service providers) and it seems to work the same as this branch. So if we do like you propose here, it is the same as removing the check.

@tabacitu could probably help here. He was the one who pushed this change https://github.com/Laravel-Backpack/theme-tabler/commit/f61f176ee964d53a83f193c5f01bd455295b1842#diff-72aafcf74cf6fd62cc5dee0fbbb8d6658b0463b1546c06d55c6c7bdcec511e6f

I didn't find nothing broken removing that check, just more view paths are registered. But then we get the correct view for the "current theme" defined in view_namespace, and it works as expected. So I think @tabacitu may have added it before finding something broken, just as a "safe guard".

If I am right here we should remove this check from the theme service providers, and let just the themes you install via composer register their paths. What will be shown is what's in view_namespace or fallback or ui as per backpack_view() helper https://github.com/Laravel-Backpack/CRUD/blob/32b9e0a4e1e3d713cd8c039e3270d1c6cad8bf49/src/helpers.php#L236

Did you found something broken by letting multiple themes register view paths @tabacitu ?

Cheers

[tabacitu]

I didn't find nothing broken removing that check, just more view paths are registered. But then we get the correct view for the "current theme" defined in view_namespace, and it works as expected. So I think @tabacitu may have added it before finding something broken, just as a "safe guard".

I yanked my brains... could NOT remember why I added that. Sorry guys.

If I am right here we should remove this check from the theme service providers

Done. I've removed it in https://github.com/Laravel-Backpack/CRUD/issues/5147 - more specifically I've commented it out here - https://github.com/Laravel-Backpack/CRUD/pull/5148/files#diff-d77f3ff44647b683d4a6a89dbe820cdb45f5e44cd6032d4c86541c2b86964d29R68-R71 so it's easy to undo if we discover it was a bad idea.

and let just the themes you install via composer register their paths. What will be shown is what's in view_namespace or fallback or ui as per backpack_view() helper https://github.com/Laravel-Backpack/CRUD/blob/32b9e0a4e1e3d713cd8c039e3270d1c6cad8bf49/src/helpers.php#L236

This... I don't understand any of this... sorry @pxpm . What did you mean here? Is this saying something we should do... is this just rephrasing... I don't understand.

[tabacitu]

Assuming Pedro didn't mean anything by that, let's close this PR in favor of https://github.com/Laravel-Backpack/CRUD/pull/5148 - we're fixing the problem upstream, basically.