Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better organisation of settings pages #154

Open
johnpuddephatt opened this issue Feb 14, 2023 · 4 comments
Open

Better organisation of settings pages #154

johnpuddephatt opened this issue Feb 14, 2023 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@johnpuddephatt
Copy link
Contributor

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 Tarpsvo added enhancement New feature or request help wanted Extra attention is needed labels Mar 16, 2023
@Tarpsvo
Copy link
Collaborator

Tarpsvo commented Mar 16, 2023

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

@johnpuddephatt
Copy link
Contributor Author

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

@codrin-axinte
Copy link

codrin-axinte commented Mar 29, 2023

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
Copy link
Collaborator

Tarpsvo commented Apr 6, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants