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

[Enhancement] Reactive options as closure for multi-select prompt #123

Closed
wants to merge 1 commit into from
Closed

[Enhancement] Reactive options as closure for multi-select prompt #123

wants to merge 1 commit into from

Conversation

GoodM4ven
Copy link

@GoodM4ven GoodM4ven commented Mar 10, 2024

  • Accepted options argument as closure
    • Turned options property to mixed type as it might hold the closure
    • Passed values property to the closure for reactivity; just like validate method
  • Added an evaluatedOptions property for caching until it's set to null
    • Created options method that evaluates, caches and returns the options all around
    • Threw an exception when all options are no longer available
    • Removed previously selected options from values when they're gone
    • Adjusted the indexing
  • Introduced a new "toggle" state to re-render without errors
    • Set the evaluatedOptions to null (resetting cache and evaluation) for toggle and error states
  • Tested for closure, reactivity, and the empty options exception
    • Ensured all tests are passing

Again, I needed this use-case (#116).

A short demo: https://github.com/laravel/prompts/assets/121377476/bfce061c-30e3-4456-93c5-84d1ec68e77a

- Accepted options argument as closure
  - Turned options property to mixed type as it might hold the closure
  - Passed values property to the closure for reactivity; just like validate method
- Added an evaluatedOptions property for caching until it's set to null
  - Created options method that evaluates, caches and returns the options all around
    - Threw an exception when all options are no longer available
    - Removed previously selected options from values when they're gone
    - Adjusted the indexing
- Introduced a new "toggle" state to re-render without errors
  - Set the evaluatedOptions to null (resetting cache and evaluation) for toggle and error states
- Tested for closure, reactivity, and the empty options exception
  - Ensured all tests are passing
Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@GoodM4ven GoodM4ven marked this pull request as ready for review March 10, 2024 14:28
@jessarcher
Copy link
Member

Hi @GoodM4ven,

Could you please explain the use case for this feature? I would imagine the UI would become confusing if the options in the list changed part-way through completing it.

Would the multisearch component be a better choice? It allows you to pass a closure that receives the text as the user is typing in order to filter down the list of options.

@jessarcher jessarcher self-requested a review March 12, 2024 05:34
@jessarcher jessarcher self-assigned this Mar 12, 2024
@GoodM4ven
Copy link
Author

GoodM4ven commented Mar 12, 2024

Yeah, sure.

It's the need for a package selection list that would initiate the necessary logic for installing and modifying them on the fly after creating a new Laravel project. I used to do all this in bash scripts (https://github.com/GoodM4ven/lara-stacker). But I thought it would be a 100 times better if those scripts only did the bare bones of what is necessary to host the sites locally and then for further package installation, it would be done with a multiselect() prompt as the starting point and then PHP later; all in a separate Laravel package (tall-stacker, soon).

So I have this code for the artisan command that would initiate the installation process, for instance:

class TallStackerCommand extends Command
{
    use HasPackages;

    public $signature = 'tall-stacker:run {--test}';

    public $description = 'Begin stacking up your TALL packages!';

    public Collection $filteredPackages;

    public Collection $selectedPackages;

    public function handle(): int
    {
        $this->initializeAllPackages();
        $this->validateAllPackages();

        if ($this->option('test')) {
            return self::SUCCESS;
        }

        $this->updateSelectedPackages();
        $this->filterAllPackages();

        $packages = multiselect(
            label: 'Choose the packages suitable for this app:',
            options: fn ($values) => $this->getFiltered($values),
            validate: fn ($values) => $this->validateSelectedPackages($values),
            validateOnToggle: true,
            default: $this->filteredPackages->filter(fn ($package, $_) => $package->isChecked)->keys(),
            required: true,
            scroll: 10,
        );

        // $this->updateSelectedPackages($packages);

        // dd($this->selectedPackages->first()->configure($this));

        return self::SUCCESS;
    }

    private function getFiltered($selectedPackageNamespaces)
    {
        $this->updateSelectedPackages($selectedPackageNamespaces);

        return $this->filteredPackages->mapWithKeys(fn ($package, $namespace) => [
            $namespace => $package->title(),
        ]);
    }

    private function filterAllPackages(): void
    {
        $this->filteredPackages = $this->allPackages->filter(function ($package, $namespace) {
            $dependenciesMet = true;
            $dependencies = $package->dependencies ?? [];

            foreach ($dependencies as $dependency) {
                if (!$this->selectedPackages->has(app($dependency)::class)) {
                    $dependenciesMet = false;
                    break;
                }
            }

            if (!$dependenciesMet) {
                // Automatically unselect the package if its dependencies are not met
                $this->selectedPackages = $this->selectedPackages->reject(function ($_, $selectedNamespace) use ($namespace) {
                    return $namespace === $selectedNamespace;
                });
            }

            return $dependenciesMet;
        })->sortBy('displayOrder');
    }

    private function updateSelectedPackages(array $namespaces = []): void
    {
        if (!empty($namespaces)) {
            $this->selectedPackages = $this->allPackages->filter(function ($package, $namespace) use ($namespaces) {
                // Check if all dependencies of the package are in the selected namespaces
                $dependencies = $package->dependencies ?? [];
                foreach ($dependencies as $dependency) {
                    if (!in_array(app($dependency)::class, $namespaces)) {
                        return false;
                    }
                }
                return in_array($namespace, $namespaces);
            });
        } else {
            // Initially, make all packages available for selection
            $this->selectedPackages = $this->allPackages;
        }

        $this->filterAllPackages(); // Re-filter packages after updating selections
    }

    private function validateSelectedPackages(array $selectedPackageNamespaces)
    {
        $response = null;
        
        if ($missingPackageMessage = $this->checkForMissingEnforcedPackages($selectedPackageNamespaces)) {
            $response = $missingPackageMessage;
        }

        if ($conflictingPackageMessage = $this->checkForConflicts($selectedPackageNamespaces)) {
            $response = $conflictingPackageMessage;
        }

        $this->updateSelectedPackages($selectedPackageNamespaces);

        return $response;
    }

    private function checkForMissingEnforcedPackages(array $selectedPackageNamespaces): null|string
    {
        $enforcedPackageNamespaces = $this->allPackages->filter(function ($package) {
            return $package instanceof EnforcedPackage || $package->name === app(Laravel::class)->name;
        })->keys();
        $missingPackages = $enforcedPackageNamespaces->diff($selectedPackageNamespaces);

        if ($missingPackages->isNotEmpty()) {
            return 'The enforced package (' . Classer::name($missingPackages->first()) . ') cannot be removed.';
        }

        return null;
    }

    private function checkForConflicts(array $selectedPackageNamespaces): null|string
    {
        $selectedPackages = $this->allPackages->filter(fn ($_, $namespace) => in_array($namespace, $selectedPackageNamespaces));

        foreach ($selectedPackages as $namespace => $package) {
            foreach ($package->conflicts ?? [] as $conflictNamespace) {
                if (in_array($conflictNamespace, $selectedPackageNamespaces)) {
                    return "There is a conflict between " . Classer::name($namespace) . " and " . Classer::name($conflictNamespace) . " packages.";
                }
            }
        }

        return null;
    }
}

The reactivity is needed since a Package has the following structure:

public function __construct(
    /** Order of installation */
    public int $order,
    /** Order of display, in the installation list */
    public int $displayOrder,
    public string $name,
    public string|null $description = null,
    /** Marked for installation by default */
    public bool $isChecked = false,
    /** Cannot be installed without these package keys */
    public array $dependencies = [],
    /** Cannot be installed alongside these package keys */
    public array $conflicts = [],
)

And where the dependencies and conflicts, for now, should re-shape the list structure according to what has been chosen or not yet... If a package has plugins, let's say, and since the list could be long, and when it's not even selected yet, you wouldn't want to show those plugins for that package UNTIL it's selected! An example would be PestPHP with its many plugins.

The other situation is for conflicts; when some packages simply cannot be installed with others, either because they're not necessary where there's another selected package that already handles their functions, or because of other compatibility issues. An example here would be choosing between Breeze and Jetstream!

What do you think?

@GoodM4ven
Copy link
Author

GoodM4ven commented Mar 12, 2024

By the way, I thought the ability to validate automatically against "conflicting packages" or anything else that might come up during the process of picking packages here was also helpful; giving instant feedback to the user instead of selecting a lot of packages in a long list and then being told about which CANNOT be picked together and so on... And that's why I PRed #121.

@taylorotwell taylorotwell marked this pull request as draft March 12, 2024 18:47
@jessarcher
Copy link
Member

Hey @GoodM4ven,

Thanks for those details! I can see how this would be useful in your case, but I don't think I'm compelled enough to bring it into Prompts and take over maintenance on it. It introduces some potentially confusing UI behaviours that I don't think I want to include as standard out of the box. For example, if you select an item at the top of the list and then scroll down and select another item that removes the top item you selected, it's not clear that something you selected has been removed because it's out of view. Also, in that scenario, the scrolling seems to break.

In the Laravel installer, we've worked around this by asking multiple questions rather than trying to fit all options into a single question. So we'll ask what starter kit you'd like, and then based on the answer, we'll prompt with a set of options relevant to the selected starter kit. You may wish to consider a similar approach for scenarios where you have mutually exclusive options. You can see some generally accepted guidelines on radio (single select) vs checkboxes (multi select) at https://www.nngroup.com/articles/checkboxes-vs-radio-buttons/.

You are, of course, free to create your own prompt with your package or application with these extra features.

Best wishes!

@jessarcher jessarcher closed this Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants