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

More control of CRUD interface elements #669

Closed
wants to merge 20 commits into from

Conversation

Antikon
Copy link
Contributor

@Antikon Antikon commented Oct 1, 2021

What are you changing/introducing

Added an $interface parameter to actionIndex() method to luya\admin\ngrest\base\Controller, so one can control which interface elements are shown and which are hidden.

Also added more complex logic for displaying the search field and the grouping and filter drop-down lists on wide screens:

  • Search ON, Group and Filter OFF => Search field will have 100% width.
  • Search OFF, Group and Filter ON => Group and Filter dropdowns will have 50% width together and aligned right.
  • Search ON, One of Group Or Filter ON => Search field will have 50% width, Group Or Filter dropdown will have the rest 50% width.
  • All ON => Search field will have 50% width, Group and Filter dropdowns will have 25% width each (the same behavior as now)

Since the methods getInterfaceSettings() and setInterfaceSettings() have been declared in the RenderCrudInterface interface, this can break custom CRUD renderers which implements RenderCrudInterface. Therefore, this PR can be postponed until version 5.0.0.

What is the reason for changing/introducing

Allow to customize CRUD interface in the controller.

QA

Q A
Is bugfix? no
New feature? yes
Breaks BC? yes
Tests pass? yes
Fixed issues #492

@nadar
Copy link
Member

nadar commented Oct 5, 2021

i saw a few small things we might change. But in general there are two things which we should discuss:

  1. Can you make an example of where people should adjust those settings? calling parent::actionIndex? what was your thought on this.
  2. Instead of an array we should pass an Object which holds the config. like CrudInterfaceSetting { $showTitle = true/false }. why? Its so much more easy to extend, document and also make settings available in IDE. So people could pass that CrudInterfaceSetting object with the desired settings including phpdocs, arrays, etc. Instead of string "modes" you could make a preconfigured settings object ModestCrudInterfaceSetting. I can make a full example if you like.

@Antikon
Copy link
Contributor Author

Antikon commented Oct 7, 2021

  1. Yes, you are right. My typical admin controller contains the following:
public function actionIndex(
        $inline = false,
        $relation = false,
        $arrayIndex = false,
        $modelClass = false,
        $modelSelection = false
    ) {
        $events = Events::findAll(1);
        $output = '';
        $output .= $this->renderPartial(
            'index',
            [
                'events' => $events
            ]
        );

        $output .= parent::actionIndex($inline, $relation, $arrayIndex, $modelClass, $modelSelection);
        return $output;
    }

I often need to add some text (instructions) or data (statistics, for example) before CRUD table.
At the same time, I would like to be able to slightly change the appearance of the table without affecting its features and logic.

  1. I totally agree with you! And an example would be very welcome

@nadar
Copy link
Member

nadar commented Oct 7, 2021

i would suggest do have a property on the base ngrest controller:

class MySuperController extends \base\Controller
{

    public $crudInterfaceConfig = [
       'class' => DefaultCrudInterfaceConfig::class,
       'intro' => 'foo bar',
       'title' => 'my title',
   ];

   public $modelClass = '....';
}

then you can pass either just the config or direct call Yii::createObject($this->crudInterfaceConfig) in actionIndex() method.

This way we can have a base CRUD interface config class which covers all those settings and options.

For your use case with extra rendering you could then also override a certain property in actionIndex():

public function actionIndex(....)
{
 $this->crudInterfaceConfig['intro'] =>  $this->renderPartial(....);
 return parent::actionIndex(...);
}

Of course this is only required when you want to use $this to provide CRUD interface config settings.

When we have decoupled all the possible crud interface config settings we could provider out of the box "dense"-mode like follows:

class DenseCrudInterfaceConfig extends CRUDInterfaceConfig
{
    .... settings for dense...like:
    $showSearch = false;
}

then you could just pass that dense config as default in your controller:

class MySuperController extends \base\Controller
{
    public $crudInterfaceConfig = DenseCrudInterfaceConfig::class;
 
}

Do you know what i mean? Inside of crud.php you can then direct access the crud config object (via
https://www.yiiframework.com/doc/api/2.0/yii-baseyii#createObject()-detail) (assuming we bind the object to $interface):

if ($interface->showSearch)  {}

@Antikon if you like i can prepare the mechanics and you fill it with "life" afterwards (adding the options, adding the if/else, adding the modes f.e)

@Antikon
Copy link
Contributor Author

Antikon commented Oct 8, 2021

@nadar, I agree with your suggestions, but I see no need for an 'intro' property. Someone may need to put something after the table, or add something more complex than a simple template before (see #491, for example). Or do you have some more substantial objection to actionIndex method overriding?

Anyway, your help with the mechanics would be greatly appreciated.

@Antikon Antikon marked this pull request as draft October 8, 2021 13:07
@nadar
Copy link
Member

nadar commented Feb 6, 2024

I am going to close this issue because:

  • No more activity

If you think this is still important or there are more/new informations. Please reopen the issue and let us know.

If this is a problem or a feature you like to discuss, join our discussion: LUYA discussions. We try to keep our issue tracker as clean as possible in order to focus on active issues.

@nadar nadar closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants