outl1ne / nova-settings

A Laravel Nova tool for editing custom settings using native Nova fields.
MIT License
271 stars 99 forks source link

Better organisation of settings pages #154

Open johnpuddephatt opened 1 year ago

johnpuddephatt commented 1 year ago

This isn't a problem, more of a suggestion, but talked through rather than jumping to the answer because I don't have the confidence to know if what I'm proposing is good!

I've noticed that having a lot of settings (fields and/or pages) results in a messy NovaServiceProvider file.

Currently in my projects I'm doing the following:

  1. Create a file for each page of settings in \App\Nova\Settings, e.g.
\App\Nova\Settings\Alert.php

<?php
namespace App\Nova\Settings;

use Laravel\Nova\Fields\Text;
use Laravel\Nova\Fields\Boolean;
use Laravel\Nova\Fields\DateTime;

class Alert
{
    public $name = "alert";

    public function fields(): array
    {
        return [
            Text::make("Message"),
            Text::make("Link"),
            Boolean::make("Enabled?"),
            DateTime::make("Display until"),
        ];
    }

    public function casts(): array
    {
        return [];
    }
}
  1. Adding the following to the boot method of AppServiceProvider.php:
$settings = [
    new \App\Nova\Settings\Alert(),
    [...etc...]
];

foreach ($settings as $setting) {
    \Outl1ne\NovaSettings\NovaSettings::addSettingsFields(
        $setting->fields() ?? [],
        $setting->casts() ?? [],
        $setting->name ?? null
    );
}

This works well enough, but I wonder if there could/should be a new method added to NovaSettings, perhaps "addSettingsPages()" to simplify the above to:

 \Outl1ne\NovaSettings\NovaSettings::addSettingsPages([
    new \App\Nova\Settings\Alert(),
    [...etc..]
]);

The new method would be pretty simple, e.g.

    public static function addSettingsPages($pages = [])
    {
        foreach ($pages as $page) {
            static::addSettingsFields($page->fields() ?? [], $page->casts() ?? [], $page->name ?? null);
        }
    }

This would certainly keep NovaServiceProvider a lot tidier. But I then started thinking that maybe we don't need to call addSettingsPages manually? Couldn't our pages be passed to the Tool directly when it's being instantiated? e.g.

 public function tools()
    {
        return [
            new \Outl1ne\NovaSettings\NovaSettings([
                new \App\Nova\Settings\Alert(),
                [...etc...]
           ])
       ]
   }

To make this work just requires a constructor on NovaSettings:

    public function __construct($pages = [])
    {
        static::addSettingsPages($pages);
    }    

Happy to create a PR if this sounds of interest, but if you have feedback or suggestions for a better approach let me know.

Tarpsvo commented 1 year ago

That definitely sounds a lot cleaner than what we have now. I would definitely be interested in a PR.

johnpuddephatt commented 1 year ago

Cool I'll create a PR when I get chance, then you can take a proper look!

codrin-axinte commented 1 year ago

I did the same thing as well. Though, mine has a bit more logic with a structure having a contract and an abstract.

Contract preview:

interface SettingsPage
{
    public function name(): string;

    public function options(): array;

    public function casts(): array;

    public function fields(): array;

    public function defaultValues(): array;

    public function guarded(): array;

    public function slug(): string;

    public function authorize(Request $request): bool;
}

Also, it has support for whitecube flexible package and feature to sync the settings with the environment file by implementing a contract SyncWithEnv.

@johnpuddephatt if you want to, we could do a merge and pick only the best features from each?

Tarpsvo commented 1 year ago

These additions do sound sweet. I'll be waiting for a PR. :)