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

[Menu] Set it more customizable? #149

Closed
cavasinf opened this issue Mar 3, 2023 · 5 comments
Closed

[Menu] Set it more customizable? #149

cavasinf opened this issue Mar 3, 2023 · 5 comments
Labels
Question Further information is requested Status: Reviewed Has staff reply/investigation

Comments

@cavasinf
Copy link
Collaborator

cavasinf commented Mar 3, 2023

I need to make a "special" menu for a client with specific needs.
To be more detail, I need:

  1. A drop-down menu
  2. Item with a button
  3. Menu separators/dividers

image

Problems:

To be able to create the screenshot menu, I've overwritten the entire TWIG menu template as there's no navbar_menu block to overwrite it from an extend:
https://github.com/kevinpapst/TablerBundle/blob/main/templates/layout-vertical.html.twig#L47-L51

Do we allows custom menu (from the subscriber), or it is a dev problem and overwriting the template is a good way to go?

@cavasinf cavasinf added Question Further information is requested Status: Needs Review Not under investigation labels Mar 3, 2023
@kevinpapst
Copy link
Owner

I don't have an answer, I can just tell you that I am using my own final class MenuItemModel implements MenuItemInterface in my project. Which you could do as well, to support the raw HTML.

You know you can add blocks where it makes sense.
I am just not a fan of adding features, which are not part of Tabler itself.
We should instead push those changes upstream if somehow possible.

I am a contributor of the Tabler repo, so if you send reasonable PRs (instead of discussions) codecalm will likely merge it. Yes, he is not very responsive, which is fine (even though it can be annoying) ... it is still a free open source project. Give it a try and see what happens.

@cavasinf
Copy link
Collaborator Author

cavasinf commented Mar 14, 2023

Sorry for the late reply, I have been running after time lately ⏲️ 🏃

(instead of discussions)

Funny part is THEY convert issues into discussions.

I am using my own final class MenuItemModel implements MenuItemInterface in my project

So you have ovewrote the menu.twig.html template?

I am just not a fan of adding features, which are not part of Tabler itself.

Agree for the separator feature that I want.
Can we at least allow raw?

Even today MenuItem has a $divider option but not used for Tabler.
Which I think comes from AdminLTE.

private bool $divider = false;

@kevinpapst
Copy link
Owner

So you have ovewrote the menu.twig.html template?

No, only the model for more features.

Can we at least allow raw?

How? General rule of thumb is: whenever you have to use raw there is something fishy going on 😁
Yes, there are legit use-cases, but not many. That |raw filter produced quite some bugs in the past for me. It is always a (big) risk, when you do not think twice about source of the data...

Make a proposal, I am not against it, if there is a good and safe use-case.

Which I think comes from AdminLTE.

Yes, very likely 🙃

@cavasinf
Copy link
Collaborator Author

cavasinf commented Mar 14, 2023

Okay, so I might need an external eye to solve my case then:
How to add disabled member of MenuItemModel AND in the getter, checks if parent is also not already disabled.

Since parent is typed for MenuItemModelInterface we are a bit stuck..

use KevinPapst\TablerBundle\Model\MenuItemModel as TablerMenuItemModel;

class MenuItemModel extends TablerMenuItemModel
{
    private bool $disabled = false;
    
    public function isDisabled(): bool
    {
        // ❌ Polymorphic call => PhpStan will not allow that
        return $this->getParent()?->isDisabled() ?? $this->disabled;
    }

    public function setDisabled(bool $disabled): void
    {
        $this->disabled = $disabled;
    }
}

ATE, I think I need to rewrite the whole MenuItemModel, and menu.html.twig to fit my needs

it is a dev problem and overwriting the template is a good way to go

@kevinpapst
Copy link
Owner

Since parent is typed for MenuItemModelInterface we are a bit stuck..

You are the boss, not your tooling 😁

    public function isDisabled(): bool
    {
        /** @var MenuItemModel $parent */
        $parent = $this->getParent();

        return $parent?->isDisabled() ?? $this->disabled; 
    }

or

    public function isDisabled(): bool
    {
        $parent = $this->getParent();
        if ($parent instanceof MenuItemModel) {
            return $parent->isDisabled();
        }

        return $this->disabled; 
    }

@cavasinf cavasinf added Status: Reviewed Has staff reply/investigation and removed Status: Needs Review Not under investigation labels Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question Further information is requested Status: Reviewed Has staff reply/investigation
Projects
None yet
Development

No branches or pull requests

2 participants