Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

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

Merged
merged 1 commit into from
May 14, 2020

Conversation

hariesnurikhwan
Copy link
Contributor

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

@hariesnurikhwan hariesnurikhwan changed the title Don't show action in checkbox selection. Don't show detached action in checkbox selection. May 13, 2020
@gobrightspot
Copy link
Owner

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

vesper8 commented May 13, 2020

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

vesper8 commented May 13, 2020

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

gobrightspot commented May 13, 2020

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

vesper8 commented May 13, 2020

@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
Copy link
Owner

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

vesper8 commented May 13, 2020

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

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

vesper8 commented May 14, 2020

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

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

@vesper8
Copy link

vesper8 commented May 14, 2020

@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
Copy link
Owner

Can you try:

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

@vesper8
Copy link

vesper8 commented May 14, 2020

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

vesper8 commented May 14, 2020

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

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

vesper8 commented May 14, 2020

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

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

Or both do not show?

@vesper8
Copy link

vesper8 commented May 14, 2020

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

vesper8 commented May 14, 2020

@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
Copy link
Owner

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants