Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3.04k stars 885 forks source link

Add header metas publish command #5552

Closed pxpm closed 1 month ago

pxpm commented 1 month ago

WHY

BEFORE - What was wrong? What was happening before this PR?

It started with the idea of adding the capability to a Backpack panel to load on mobile devices as a "native app" when bookmarked on homepage in https://github.com/Laravel-Backpack/CRUD/pull/3318

While doing some information gathering to take the decision to do something about that PR, I found that by default Backpack does not even has a simple favicon meta 😞

That needed to change πŸ‘

AFTER - What is happening after this PR?

Developers can now publish a header_metas file along with some Backpack images as placeholders so that their apps have a favicon, display as an app in mobile, have colored navigation bar when displayed.

image image image

HOW

How did you achieve that, in technical terms?

Added a command: backpack:publish-header-metas that will ask some questions to the developer, replace the stubs with the provided values and publish them.

image

Is it a breaking change?

NO, this needs https://github.com/Laravel-Backpack/theme-tabler/pull/185

dimer47 commented 2 weeks ago

This is a fantastic feature @pxpm ! But there is a slight incompatibility between the commands php artisan backpack:publish-header-metas and php artisan backpack:install -q -n.

When using the php artisan backpack:publish-header-metas command, it prompts for the installation location of the assets. However, by default, the php artisan backpack:install -q -n command installs everything in the public/ directory.

The BackpackServiceProvider.php file located in vendor/backpack/crud/src/BackpackServiceProvider.php contains the following code:

public function publishFiles()
{
    ...
    $backpack_public_assets = [__DIR__.'/public' => public_path()];
    ...
}

Should the assets really be deployed by default during the installation? Or wouldn't it be better to prompt for the installation location of the assets during the installation, with the default value set to /public?

May I create a PR to implement this request, or do you have a different perspective on the matter?

pxpm commented 2 weeks ago

Hey @dimer47 nice catch! Completely missed that. 😞

I think the correct thing to do is removing that $backpack_public_assets from the publish command as there is nothing in that folder that we would like to publish.

If you have time to create the PR, please go ahead, I will merge and tag a new version right way. πŸ‘

dimer47 commented 6 days ago

Hey @pxpm, sorry for the delay! πŸ˜…

I've gone ahead and created the PR as requested. I've removed the $backpack_public_assets from the publishFiles method, so no assets will be published during the backpack:install command anymore. This ensures that asset deployment is only triggered by the backpack:publish-header-metas command, just as we discussed.

Merge Request : https://github.com/Laravel-Backpack/CRUD/pull/5619

Thank you for your patience. Looking forward to the merge. 😊