gobrightspot / nova-detached-actions

A Laravel Nova tool to allow for placing actions in the Nova toolbar detached from the checkbox selection mechanism.
MIT License
168 stars 50 forks source link

Don't show detached action in checkbox selection. #5

Closed hariesnurikhwan closed 4 years ago

hariesnurikhwan commented 4 years ago

Since detached actions is independent, it makes sense for us not to show it in checkbox selection.

gobrightspot commented 4 years ago

Thanks for your PR! This is a really Interesting suggestion.

In the case of exports, you definitely want to allow actions to export both, all data and only selected data, so you would have to add shownOnIndex() to every export. But if you're doing more actual detached actions, where you don't care about the selected rows under any circumstances, then you have more work adding exceptOnIndex() on your actions.

What's your use case where defaulting to 'not shown on index' makes more sense?

vesper8 commented 4 years ago

@gobrightspot It seems that these visibility options have no effect on detached actions?

I am trying to add it to a test detached action like so:

    public function actions(Request $request)
    {
        return [
            (new App\Nova\Actions\LogUsers)->onlyOnDetail(),
        ];
    }

And it has no effect.. but if I add the same ->onlyOnDetail() to other actions it does then remove them from the actions dropdown after selecting 1 or more rows

vesper8 commented 4 years ago

Adding any combination of these on the action class itself also has no effect at all..

class LogUsers extends DetachedAction
{
    use InteractsWithQueue, Queueable;

    public $showOnIndex = false;
    public $exceptOnIndex = true;
    public $onlyOnDetail = true;
gobrightspot commented 4 years ago

@vesper8 Thanks for the info. I haven't looked at any of that yet to be honest.

If you change the DetachedAction class back to a regular Nova Action class and you pass the following withMeta(), does that make any difference for you?

     /**
      * Get the actions available for the resource.
      *
      * @param  \Illuminate\Http\Request  $request
      * @return array
      */
     public function actions(Request $request)
     {
         return [
             (new LogUsers)->withMeta([
                 'detachedAction' => true,
                 'label' => 'Log Users'
             ]),
         ];
     }
vesper8 commented 4 years ago

@gobrightspot yes that does the trick.. if instead of extending the class with Brightspot\Nova\Tools\DetachedActions\DetachedAction you extend it as a normal nova action, and then you pass

->withMeta([
    'detachedAction' => true,
    'label' => 'Log Users'
]),

Then it will actually respect public $showOnIndex = false;

Although public $exceptOnIndex = true; doesn't do anything.. which you would except to work since public $showOnIndex = false; works.. but maybe I'm just confused how those are supposed to work

gobrightspot commented 4 years ago

@vesper8 hopefully that solution will help you out for now but I definitely want to figure out how to make the DetachedAction respect the showOn/exceptOn properties. I'll take a look this evening and let you know how it goes.

vesper8 commented 4 years ago

thank you kindly : )

I was very happy that I figured out recently how to create a custom resource tool that adds a button to the taskbar which can trigger a modal with a form.. I was like yeah! This will come in handy! Then I find out about your package hours later and it does exactly what I wanted but oh so simpler!! =D

gobrightspot commented 4 years ago

@vesper8 @hariesnurikhwan So, it seems there are quite a few variations we're now talking about in this thread so I've tried to break it down to what works for me so far in an isolated Nova install with only this package installed.

Scenario 1: Action should display in the Toolbar (next to 'Create') Action should not display on the Index view Action should not display on the Detail view

...
    public $onlyOnIndex = false;
    public $showOnIndex = false;
    public $onlyOnDetail = false;
    public $showOnDetail = false;
...

Scenario 2: Action should display in the Toolbar (next to 'Create') Action should display on the Index view Action should not display on the Detail view

...
    public $onlyOnIndex = true;
    public $showOnIndex = true;
    public $onlyOnDetail = false;
    public $showOnDetail = false;
...

Scenario 3: Action should display in the Toolbar (next to 'Create') Action should not display on the Index view Action should display on the Detail view

...
    public $onlyOnIndex = false;
    public $showOnIndex = false;
    public $onlyOnDetail = true;
    public $showOnDetail = true;
...

Scenario 4: Action should display in the Toolbar (next to 'Create') Action should display on the Index view Action should display on the Detail view

...
    public $onlyOnIndex = true;
    public $showOnIndex = true;
    public $onlyOnDetail = true;
    public $showOnDetail = true;
...

Okay, so all of the above scenarios work for me.

This PR is about setting the default visibility of the Action to Scenario 1.

I'm not 100% sure that's the ideal default yet but if we get enough votes for, I'm happy to make it happen.

As a side note, one thing we currently cannot do is use the default Nova Action visibility properties to effect set the Detached action button to not shown on the Index view. Which may or may not be an issue.

Another issue, and @vesper8 maybe this was your particular issue, is that we don't support placing the Detached action button on the Detail view toolbar (it is possible to add it to the action dropdown)

vesper8 commented 4 years ago

I didn't have any issues at all.. after reading this thread I learned of the ability to set visibility and after trying it I saw that it didn't work so I brought it up.

One possible issue may occur when defining custom detached actions and having them properly act either globally or on a subset of selected rows. This works perfectly with the DownloadExcel example but I imagine with a custom detached action it may not "just work" and would require something special to be done, thus I would probably want to hide it from the actions dropdown and only have it work globally. In any case I haven't been able to test any of this because the handle method isn't being executed.. I guess that will change soon when you tag a new release : )

gobrightspot commented 4 years ago

@vesper8 Just tagged 1.0.3, I would appreciate your feedback whether you can or cannot get the handle method to execute.

vesper8 commented 4 years ago

@gobrightspot am now using 1.0.4 and now that you've removed the DetachedActions tool, I can no longer get my detacted action button to appear at all (next to the create button) no matter what I do. I tried with withMeta and extending the class with the normal Nova Action, without withMeta and extending the class with Brightspot\Nova\Tools\DetachedActions\DetachedAction, and tried multiple combinations of

    public $onlyOnIndex = false;
    public $showOnIndex = false;
    public $onlyOnDetail = false;
    public $showOnDetail = false;

As well as not setting them at all, no matter what I do, I can get it to appear on the index (when selecting a row and using the actions dropdown) but nothing I do has gotten it to show next to the create button like it previously was

gobrightspot commented 4 years ago

Can you try:

<?php
    /**
     * Indicates if this action is only available on the custom index toolbar.
     *
     * @var bool
     */
    public $showOnIndexToolbar = true;
vesper8 commented 4 years ago

The DownloadExcel no longer shows up next to the create button neither, but it does show up when selecting a row

            (new \App\Nova\Actions\LogUsers)
                ->withMeta([
                    'detachedAction' => true,
                    'label' => 'Log Users',
                    'showOnIndexToolbar' => true,
                ]),

            (new \Maatwebsite\LaravelNovaExcel\Actions\DownloadExcel)
                ->withHeadings()
                ->askForWriterType()
                ->withMeta([
                    'detachedAction' => true,
                    'label' => 'Export Users',
                    'showOnIndexToolbar' => true,
                ])
                ->confirmButtonText('Export'),
vesper8 commented 4 years ago

Tried

    public $showOnIndexToolbar = true;

It has no effect, tried with withMeta and extending the class with the normal Nova Action, without withMeta and extending the class with Brightspot\Nova\Tools\DetachedActions\DetachedAction

gobrightspot commented 4 years ago

Weird, it's working fine on 1.0.4 for me. I don't have time to dig in right now but I will get to the bottom of this. If not today, then tomorrow.

vesper8 commented 4 years ago

Yea very weird.. I tried disabling everything else in my resource (all filters, lenses, other actions). I also tried clearing all caches, and tried deleting the vendor folder and reinstalling all dependencies.

To rule out that it's not something to do with my action I only left

            (new \Maatwebsite\LaravelNovaExcel\Actions\DownloadExcel)
                ->withHeadings()
                ->askForWriterType()
                ->withMeta([
                    'detachedAction' => true,
                    'label' => 'Export Users',
                    'showOnIndexToolbar' => true,
                ])
                ->confirmButtonText('Export'),

And it's not showing up, it does show when you select a row in the action dropdown though

gobrightspot commented 4 years ago

Just to clarify, are you saying the DetachedAction does show but the DownloadExcel + withMeta() does not show?

Or both do not show?

vesper8 commented 4 years ago

neither show no matter what I do (next to the create button) but both show in the actions dropdown when selecting a row

I went back to 1.0.3 and initially they didn't show either, but after re-adding the DetachedActions to the tools in NovaServiceProvider then they showed again next to the create button

But can't do that on 1.0.4 since you removed that tool

vesper8 commented 4 years ago

@vesper8 Just tagged 1.0.3, I would appreciate your feedback whether you can or cannot get the handle method to execute.

Just want to confirm that in 1.0.3 the handle method is now executing

gobrightspot commented 4 years ago

Hi @vesper8, I've released a new version, I still can't replicate your issue on my end. Can you create a separate issue with as much information as possible, including full code examples of your action, resource and errors (if any) you are seeing.