CRUD icon indicating copy to clipboard operation
CRUD copied to clipboard

[Bug] Multi level menus not consistent across themes & inconsistent behaviour.

Open piethonkoop opened this issue 1 year ago • 9 comments

Bug report

What I did

After upgrade to v6, changed the menu structure to the newer blade directives + changed to tabler theme

The current menu structure contains 2 submenus under different headings So: A A1 A2 A2.1 A2.2 B B1 B1.1 B1.2 B2

What I expected to happen

menu functionality remains the same across both themes

What happened

with tabler theme, the second submenu (B1 in the example above) does not open although the html is there. Instead, the original menu collapses (so: click on B, submenu B1/B2 opens; click on B1, B closes)

If I try the same with A, it all works. So: Click on A opens A1/A2, click on A2 opens A2.1/A2.2 menu)

switching back to coreuiv2 it all works as expected.

Is it a bug in the latest version of Backpack?

After I run composer update backpack/crud the bug... is it still there?

Yes

Backpack, Laravel, PHP, DB version

When I run php artisan backpack:version the output is:

PHP VERSION:

PHP 8.2.13 (cli) (built: Nov 24 2023 08:46:50) (NTS) Copyright (c) The PHP Group Zend Engine v4.2.13, Copyright (c) Zend Technologies with Zend OPcache v8.2.13, Copyright (c), by Zend Technologies

LARAVEL VERSION:

10.39.0.0

BACKPACK PACKAGE VERSIONS:

backpack/basset: 1.2.2 backpack/crud: 6.5.1 backpack/generators: v4.0.2 backpack/permissionmanager: 7.1.1 backpack/pro: 2.0.20 backpack/settings: 3.1.0 backpack/theme-coreuiv2: 1.2.2 backpack/theme-tabler: 1.2.0

piethonkoop avatar Jan 07 '24 14:01 piethonkoop

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

  • Bug Reports, Feature Requests - Github Issues (here);
  • Quick help (How do I do X) - Gitter Chatroom;
  • Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the backpack-for-laravel tag;
  • Showing off something you've made, asking for opinion on Backpack/Laravel matters - Reddit;

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

-- Justin Case The Backpack Robot

welcome[bot] avatar Jan 07 '24 14:01 welcome[bot]

HI @piethonkoop can you share with me the menu example, so i can replicate the error on my side.

Thanks.

jcastroa87 avatar Jan 08 '24 20:01 jcastroa87

Hi @jorgetwgroup ,

Code example below. Trimmed down as much as possible of course and matched the titles to the example above

Thanks for your help :)

P.

`

@if ( Illuminate\Support\Facades\Auth::check()) <x-backpack::menu-item title=" Home" icon="la la-home" :link="backpack_url('dashboard')" />

@if ( Illuminate\Support\Facades\Auth::user()?->is_admin)
    @can('ViewAdminInfo')
        <x-backpack::menu-dropdown title=" Pricing" icon="la la-group">
            <x-backpack::menu-dropdown-item title=" A" icon="las la-euro-sign" :link="backpack_url('a')" />
            @can('ViewNextPrices')
                <x-backpack::menu-dropdown title=" A1" icon="la la-group" nested="true">
                    <x-backpack::menu-dropdown-item title=" A1.1" icon="las la-euro-sign" :link="backpack_url('a11')" />
                    <x-backpack::menu-dropdown-item title=" A1.2" icon="las la-euro-sign" :link="backpack_url('a12')" />
                </x-backpack::menu-dropdown>
            @endcan            
		</x-backpack::menu-dropdown>

    @endcan

    @can('MoveToAdmin')
        <x-backpack::menu-dropdown title=" B" icon="la la-group">
            <x-backpack::menu-dropdown-item title=" B1" icon="las la-caret-right" :link="backpack_url('b1')" />
            <x-backpack::menu-dropdown-item title=" B2" icon="las la-caret-right" :link="backpack_url('b2')" />
			<x-backpack::menu-dropdown title=" B3" icon="la la-group">
				<x-backpack::menu-dropdown-item title=" B3.1" icon="las la-caret-right" :link="backpack_url('b31')" />
				<x-backpack::menu-dropdown-item title=" B3.2" icon="las la-caret-right" :link="backpack_url('b32')" />
			</x-backpack::menu-dropdown>
        </x-backpack::menu-dropdown>
    @endcan
@endif

@endif

`

piethonkoop avatar Jan 08 '24 20:01 piethonkoop

upon testing with a larger example (more menu dropdowns at highest level), it seems there is more wrong: dropdowns without submenu get closed when switching to different menu, the ones with submenu do not auto close others.

Can't really see why that is happening. Again: only with the tabler theme, not with coreuiv2.

Thanks for your help,

Piet

piethonkoop avatar Jan 08 '24 22:01 piethonkoop

Hello @piethonkoop sorry for delay.

I confirm bug, i will assign to our patch team.

Normal menu on "coreuiv2", on "coreuiv4" work ok too

Screenshot 2024-01-12 at 19 01 02

Bug on "tabler", menu didnt show correctly

Screenshot 2024-01-12 at 19 01 58 Screenshot 2024-01-12 at 19 04 41

Cheers.

jcastroa87 avatar Jan 12 '24 22:01 jcastroa87

Hi,

I'm also affected by this bug and can't understand why. This is critical, postponing our upgrade to backpack6.

Any news about a patch soon ? Maybe someone else who looked into it has some tips to help fix it on our side while we wait for a fix ?

Antonin-netalis avatar Feb 13 '24 14:02 Antonin-netalis

Hey @Antonin-netalis we will have a look at this, I couldn't prioritize this when this only affect one theme and in specific scenarios like multi level menus.

We have issues that affect all the users, and those are the ones we put in top priority. As @jcastroa87 pointed out, the issue does not happen in neither of the core-ui themes, so if this is the issue preventing you from upgrade, you can safely upgrade to v6 using coreui-v2 (the same you have in v5) and then if you want to, change the theme to tabler. Actually that is what we recommend in our upgrade guide https://backpackforlaravel.com/docs/6.x/upgrade-guide#step-3.2 , first upgrade using coreui-v2, and then do the theme change, as two separate processes.

I will try to pick this up sometime soon, but without promises, it will depend on my work on other issues (more important in my opinion) than this one. Resources/Time are not infinite, so we need to allocate them properly.

If you find the issue and want to contribute with the fix, feel free to submit a PR, themes and crud are open source packages so any contribution is highly appreciated.

Cheers

pxpm avatar Feb 13 '24 16:02 pxpm

Hey @Antonin-netalis we will have a look at this, I couldn't prioritize this when this only affect one theme and in specific scenarios like multi level menus.

We have issues that affect all the users, and those are the ones we put in top priority. As @jcastroa87 pointed out, the issue does not happen in neither of the core-ui themes, so if this is the issue preventing you from upgrade, you can safely upgrade to v6 using coreui-v2 (the same you have in v5) and then if you want to, change the theme to tabler. Actually that is what we recommend in our upgrade guide https://backpackforlaravel.com/docs/6.x/upgrade-guide#step-3.2 , first upgrade using coreui-v2, and then do the theme change, as two separate processes.

I will try to pick this up sometime soon, but without promises, it will depend on my work on other issues (more important in my opinion) than this one. Resources/Time are not infinite, so we need to allocate them properly.

If you find the issue and want to contribute with the fix, feel free to submit a PR, themes and crud are open source packages so any contribution is highly appreciated.

Cheers

Hey @pxpm . Thanks for the quick answer. I understand that you have other priorities and that other themes are working fine but we really want to use tabler for different reasons.

Anyway I ended up just rewriting the menu in html without using components since we don't need different layouts. I was afraid at first that the theme was completely affected by the issue and not just the menu components. So not as critical as I thought for us, it will do for now. I couldn't understand what causes the issue but if I do I will gladly submit a PR.

Antonin-netalis avatar Feb 13 '24 17:02 Antonin-netalis

Hi @pxpm

While trying to find a way around this without doing the menus in html, i found another one: if you insert a separator in a submenu, the following happens:

  • the menu breaks off
  • the separator line is inserted at top level, not within the submenu
  • the remainder of the menu is mangled further down the screen with bullets from the original list item (+ the icon).

choice of layout (horizontal/vertical) has no impact, both are mangled.

You said:

Actually that is what we recommend in our upgrade guide https://backpackforlaravel.com/docs/6.x/upgrade-guide#step-3.2 , > first upgrade using coreui-v2, and then do the theme change, as two separate processes.

That is exactly what we did.

Short conclusion: the components used for tabler are not production ready.

Given that tabler is the default theme in v6, I ask you to reconsider the priority. Would not want to see people turned away from a further excellent product :)

Thanks

piethonkoop avatar Feb 18 '24 11:02 piethonkoop

Thanks for the feedback and for expressing your concerns 🙏

I think you made a very good point there, I will update this issue in priority so that we tackle this next week. 👍

Cheers

pxpm avatar Feb 18 '24 13:02 pxpm

One small other thing while you're at it: if you can define the image both in cover and illustration as a config item that would be great too. It prevents from overriding the blade file. Saves you problems with updates / upgrades..

Thanks

piethonkoop avatar Feb 18 '24 16:02 piethonkoop

Hey @piethonkoop I've just fixed the multi level menu issue in tabler 1.2.1. You should be able to get the fix with a composer update.

One small other thing while you're at it: if you can define the image both in cover and illustration as a config item that would be great too.

What blade files are you talking ? Can you give me an example ? Better yet, I will close this issue related with the multi level, and can you please open a separate issue giving more details on this ?

Thanks again for the explanation and sticking with me to fix it 🙏

pxpm avatar Feb 22 '24 18:02 pxpm