IBX-6279: Multilevel popup menu #897

tischsoic · 2023-09-07T07:28:14Z

Question	Answer
Tickets	https://issues.ibexa.co/browse/IBX-6279
Bug fix?	no
New feature?	yes/no
BC breaks?	no
Tests pass?	yes
Doc needed?	yes
License	GPL-2.0

MultilevelPopupMenu

This is a new component which is context-like menu.

twig example

Result

Code

{{ include('@ibexadesign/ui/component/multilevel_popup_menu/multilevel_popup_menu.html.twig', {
    groups: [
        {
            id: 'default',
            items: [
                {
                    label: 'Label 1',
                    action_attr: { href: path('logout') },
                    sublabel: 'default',
                },
                {
                    label: 'Label 2',
                },
            ],
        },
        {
            id: 'second',
            items: [
                {
                    label: 'Label 3.1',
                },
                {
                    label: 'Label 3.2',
                    sublabel: 'default',
                    branch: {
                        groups: [
                            {
                                id: 'default',
                                items: [
                                    {
                                        label: 'SubLabel 1',
                                        branch: {
                                            groups: [
                                                {
                                                    id: 'group1',
                                                    items: [
                                                        {
                                                            label: 'SubSubLabel 1',
                                                        },
                                                        {
                                                            label: 'SubSubLabel 2',
                                                        },
                                                    ],
                                                },
                                            ],
                                        }
                                    },
                                    {
                                        label: 'SubLabel 2',
                                        branch: {
                                            groups: [
                                                {
                                                    id: 'group2',
                                                    items: [
                                                        {
                                                            label: 'SubSubLabel 1',
                                                        },
                                                        {
                                                            label: 'SubSubLabel 2',
                                                        },
                                                    ],
                                                },
                                            ],
                                        }
                                    },
                                ]
                            }
                        ]
                    },
                },
                {
                    label: 'Label 4',
                },
            ]
        }
    ],
}) }}

JS

TODO

Checklist:

Coding standards ($ composer fix-cs)
Ready for Code Review

tischsoic · 2023-09-08T07:57:49Z

src/bundle/Resources/public/scss/_edit-header.scss

@@ -8,7 +8,7 @@
    border-top-left-radius: $ibexa-border-radius;
    border-top-right-radius: $ibexa-border-radius;
    transition: all $ibexa-admin-transition-duration $ibexa-admin-transition, border-bottom-width 0;
-    z-index: 2;
+    z-index: 1050;


Note: restored after mistakenly deleted in: https://github.com/ibexa/admin-ui/pull/886/files#diff-4b3ad942bc4ac8d54e34e612e6b4708697317a9ae52700f12e676e2ebc1baa7b

Gengar-i · 2023-09-08T08:45:54Z

src/bundle/Resources/public/scss/_popup-menu.scss

-                }
-            }
-        }
+    &--no-absolute {


Maybe instead of no-absolute -> initial-pos or static-pos

src/bundle/Resources/public/js/scripts/admin.context.menu.js

src/bundle/Resources/public/js/scripts/admin.multilevel.popup.menu.js

GrabowskiM · 2023-09-08T13:17:15Z

src/bundle/Resources/public/js/scripts/core/multilevel.popup.menu.js

+            }
+        }
+
+        isOurBranch(branch) {


what does Our mean in this case?

Whether this branch belongs to this menu.

src/bundle/Resources/public/scss/_split-button.scss

GrabowskiM · 2023-09-08T13:20:55Z

src/bundle/Resources/public/scss/_split-button.scss

+
+    &__split {
+        display: none;
+        width: calculateRem(0.5px);


Do we want to use half pixels? I think there were some problems between browsers/systems using it, but I'm not sure now

AFAIR I've done this for the retina to look better and AFAIK we use half pixels.
Actually, it will be served to the browser in rem where we use non-integer values all the time.

GrabowskiM · 2023-09-08T13:22:16Z

src/bundle/Resources/public/scss/_popup-menu.scss

        display: none;
+        position: absolute;
+        right: calculateRem(8px);
+        top: calc(50% - calculateRem(16px) / 2);


I think that does not work and you have to use ${calculateRem(xxx) inside calc

GrabowskiM · 2023-09-08T13:22:23Z

src/bundle/Resources/public/scss/_popup-menu.scss

+            content: '';
+            border-top: calculateRem(1px) solid $ibexa-color-light;
+            display: flex;
+            width: calc(100% - calculateRem(16px));


here as well

Gengar-i · 2023-09-08T13:34:00Z

src/bundle/Resources/public/js/scripts/admin.context.menu.js

@@ -21,40 +25,109 @@
        },
        onAdapted: (visibleItems, hiddenItems) => {
            const hiddenButtonsIds = [...hiddenItems].map((item) => item.querySelector('.ibexa-btn').id);
+            const topBranchItems = multilevelPopupMenu.getBranchItems(topBranch);


Is branch item like all of the items in current menu?
what is topBranchItems then? ParentItems of currentBranch?
Maybe parentBranchItems?
Or only like pinned items on top?

I think I don't understand these questions, could you be more specific/precise? :)

I just dont understand what is topBranch items :D

src/bundle/Resources/public/js/scripts/admin.context.menu.js

Gengar-i · 2023-09-08T13:41:31Z

src/bundle/Resources/public/js/scripts/admin.context.menu.js

+            const relatedMainBtnId = mainBtn.id;
+            const mainBtnLabel = mainBtn.querySelector('.ibexa-btn__label').textContent;
+            const {
+                alternativeMainBtnLabel: mainBtnAlternativeLabel,


is it bc you are already using this name in scope?

I wanted this name to be more consistent with names in the upper scope.

Gengar-i · 2023-09-08T13:44:34Z

src/bundle/Resources/public/js/scripts/core/multilevel.popup.menu.js

@@ -0,0 +1,385 @@
+(function (global, doc, ibexa, Popper) {
+    class MultilevelPopupMenu {
+        constructor(config) {


what about destructuring config?

We tend to not destructure configs in our codebase so that it is more clear in the constructor which thing comes from it.

src/bundle/Resources/public/js/scripts/core/split.btn.js

Gengar-i · 2023-09-08T13:53:28Z

src/bundle/Resources/public/js/scripts/core/multilevel.popup.menu.js

+            return newBranchElement;
+        }
+
+        // eslint-disable-next-line no-unused-vars


can u explain why we need this here?
Cannot we just pass instead of arguments data and this variable?

arguments is better in a case e.g. this class is overwritten.

Gengar-i · 2023-09-08T13:55:51Z

src/bundle/Resources/public/js/scripts/core/multilevel.popup.menu.js

+
+            branchElement.classList.toggle('ibexa-popup-menu--hidden', !shouldBeExpanded);
+
+            if (branchElement === topBranch) {


it should work

lucasOsti · 2023-09-13T07:37:29Z

src/bundle/Resources/public/js/scripts/admin.context.menu.js

+        ),
+    ];
+    const popupMenuElement = adaptedItemsContainer.querySelector('.ibexa-context-menu__item--more .ibexa-multilevel-popup-menu');
+    const showPopupButton = adaptedItemsContainer.querySelector('.ibexa-btn--more');


Sometimes you use btn (relatedBtn) and sometimes button (showPopupButton) in your variable names, I think it would be good to stick to one version

lucasOsti · 2023-09-13T07:44:14Z

src/bundle/Resources/public/js/scripts/admin.context.menu.js

+                alternativeToggleLabel,
+            } = menuButton.dataset;
+            const subitemsBtns = [...splitBtn.branchElement.querySelectorAll('.ibexa-popup-menu__item-content')];
+


Suggested change

lucasOsti · 2023-09-13T08:13:30Z

src/bundle/Resources/public/scss/_main-menu-popup.scss

+        }
+    }
+
+    .ibexa-main-menu-popup &__group-name {


Why not

Suggested change

.ibexa-main-menu-popup &__group-name {

&__group-name {

or

Suggested change

.ibexa-main-menu-popup &__group-name {

& &__group-name {

… z-index

Co-authored-by: Marek Nocoń <mnocon@users.noreply.github.com>

sonarcloud · 2023-09-26T13:47:07Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
28 Code Smells

No Coverage information
12.5% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

tischsoic self-assigned this Sep 7, 2023

tischsoic mentioned this pull request Sep 7, 2023

IBX-6279: Simplify actions in create and edit modes #862

Merged

github-actions bot assigned ciastektk, mikadamczyk, Nattfarinn, ViniTou and dew326 Sep 7, 2023

tischsoic commented Sep 8, 2023

View reviewed changes

tischsoic marked this pull request as ready for review September 8, 2023 08:25

tischsoic requested review from GrabowskiM, dew326, lucasOsti and Gengar-i September 8, 2023 08:25

Gengar-i reviewed Sep 8, 2023

View reviewed changes

Gengar-i approved these changes Sep 8, 2023

View reviewed changes

GrabowskiM requested changes Sep 8, 2023

View reviewed changes

Gengar-i self-requested a review September 8, 2023 13:24

Gengar-i suggested changes Sep 8, 2023

View reviewed changes

tischsoic requested review from Gengar-i and GrabowskiM September 10, 2023 14:09

tischsoic force-pushed the multilevel-popup-menu branch 2 times, most recently from b9de87f to 040945a Compare September 11, 2023 11:11

lucasOsti approved these changes Sep 13, 2023

View reviewed changes

Gengar-i approved these changes Sep 13, 2023

View reviewed changes

dew326 approved these changes Sep 14, 2023

View reviewed changes

GrabowskiM approved these changes Sep 18, 2023

View reviewed changes

tischsoic added 4 commits September 20, 2023 14:44

IBX-6279: Simplify actions in edit mode

959d51a

multilevel popup menu in context menu - initial implementation

3689a1a

frontend improvements

8a60db8

use ibexa-popup-menu in ibexa-multilevel-popup-menu

d2aa2db

tischsoic added 17 commits September 20, 2023 14:44

popup menu adjustments

4d5855a

open multilevel-popup-menu branches on hover

0e4f12a

add possibility to generate links in mlpm

6008111

context menu fixes

0832ab5

Added groups in popup menu

f6b61e2

Add generateMenu to MultilevelPopupMenu; other fixes

ce78061

processBranchOnInitAfter and processItemOnInitAfter

47ff539

add groups is defined check

9cdedc4

fix clickoutside when no branch in MPM

124cd09

Preview in context menu fixes - front

f6023e7

popup CSS fix

23ab763

Remove unnecessary things from layout

1ee9110

prettier; etc

381e831

remove console.log

584ea20

js fix

fc8ce85

Restore accidentally deleted z-indexes (in other PR); fix ibexa-popup…

e595ba5

… z-index

after CR

b23ad56

tischsoic force-pushed the multilevel-popup-menu branch from 040945a to b23ad56 Compare September 20, 2023 12:44

close on item click

e4d0d14

bogusez approved these changes Sep 26, 2023

View reviewed changes

bogusez added QA approved Ready for MERGE labels Sep 26, 2023

IBX-6279: Simplify actions in create and edit modes (#862)

ceba506

Co-authored-by: Marek Nocoń <mnocon@users.noreply.github.com>

dew326 merged commit 659fa4f into main Sep 26, 2023
8 of 12 checks passed

dew326 deleted the multilevel-popup-menu branch September 26, 2023 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IBX-6279: Multilevel popup menu #897

IBX-6279: Multilevel popup menu #897

tischsoic commented Sep 7, 2023

tischsoic Sep 8, 2023

Gengar-i Sep 8, 2023

GrabowskiM Sep 8, 2023

tischsoic Sep 10, 2023

GrabowskiM Sep 8, 2023

tischsoic Sep 10, 2023

GrabowskiM Sep 8, 2023

tischsoic Sep 10, 2023

GrabowskiM Sep 8, 2023

tischsoic Sep 10, 2023

Gengar-i Sep 8, 2023

tischsoic Sep 10, 2023 •

edited

Gengar-i Sep 11, 2023

Gengar-i Sep 8, 2023

tischsoic Sep 10, 2023

Gengar-i Sep 8, 2023

tischsoic Sep 10, 2023

Gengar-i Sep 8, 2023

tischsoic Sep 10, 2023

Gengar-i Sep 8, 2023

lucasOsti Sep 13, 2023

lucasOsti Sep 13, 2023

lucasOsti Sep 13, 2023

sonarcloud bot commented Sep 26, 2023


		branchElement.classList.toggle('ibexa-popup-menu--hidden', !shouldBeExpanded);

		if (branchElement === topBranch) {

IBX-6279: Multilevel popup menu #897

IBX-6279: Multilevel popup menu #897

Conversation

tischsoic commented Sep 7, 2023

MultilevelPopupMenu

twig example

Result

Code

JS

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tischsoic Sep 10, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 26, 2023

tischsoic Sep 10, 2023 •

edited