@yurabakhtin I'm a bit ambivalent about method chaining. On the one hand, it's very nice, but on the other hand, it's confusing when there are too many options and it's unclear which ones are required and which ones are optional.

In general, imo it would be best to enforce mandatory (or very typical) attributes via the constructor/create method and make "optional" attributes available via method chaining.

$this->addEntry(MenuLink::create(Yii::t('AdminModule.base', 'Spaces'), ['/admin/space'])
    ->id('spaces')
    ->icon('dot-circle-o')
    ->sortOrder(400)
    ->active('admin', 'space')
    ->visible(Yii::$app->user->can([ManageSpaces::class, ManageSettings::class]));

The second route parameter MenuLink::create(string $label, ?string|array $route) should also be allowed to be null here, but should have to be explicitly set to null.

What do you think about this change?

@luke- Ok, for me it is enough too, main idea is to replace the array code to functions, because IDE suggests what methods are allowed for the object when I write -> but it is not fast to go to inside the class and find what properties the class has when it is used as array options.

Maybe we should use the method name as instance instead of create because I see this name is used more often.

The second route parameter MenuLink::create(string $label, ?string|array $route) should also be allowed to be null here, but should have to be explicitly set to null.

I see MenuLink is always used with URL, (except of this single case where property link is directly built as button, but we can keep it as is, without changing) so we can make the second param $route as mandatory string|array. Or when do you think it can be used with null?

@luke- Ok, for me it is enough too, main idea is to replace the array code to functions, because IDE suggests what methods are allowed for the object when I write -> but it is not fast to go to inside the class and find what properties the class has when it is used as array options.

👍

Maybe we should use the method name as instance instead of create because I see this name is used more often.

::instance is fine too!

The second route parameter MenuLink::create(string $label, ?string|array $route) should also be allowed to be null here, but should have to be explicitly set to null.

I see MenuLink is always used with URL, (except of this single case where property link is directly built as button, but we can keep it as is, without changing) so we can make the second param $route as mandatory string|array. Or when do you think it can be used with null?

Maybe there are some JS links, so I think we should allow null to avoid problems in future.

@luke- New changes a91a2fe.
Please note one case where I decided to define URL in the separate method url() instead of the second param because the link with title is long, and maybe. we could use this way for all others, i.e. one option is on single line:

        $this->addEntry(MenuLink::instance(Yii::t('AdminModule.base', 'Modules') . $this->getMarketplaceUpdatesBadge())
            ->url(['/admin/module'])

If the method name MenuLink::instance() is long we can name it as MenuLink::to(),
same method name we have to the class humhub\widgets\bootstrap\Link.

@yurabakhtin Thanks, looks good! If the PR isn't urgent, I'd like to put it on hold. We also wanted to take a closer look at the link/button classes, and it really makes sense to standardize them here as well.

@luke- Yes, it is not urgent, but it would be good to refactor the menu link adding in future.