An other way to open menu #308

gwenandres · 2024-02-07T17:23:13Z

Hello,
with @sylvainbeo we propose a new way to open main menu.

If you add appMenuCompact in TopBar cfg, main menu is open in compact mode.
On hover you will have all details .

I hope we can propose this for everyone.

Have a good day

manisandro · 2024-02-08T08:02:45Z

Thanks, I like the idea! I believe it should be restricted to desktop though?

manisandro · 2024-02-08T13:39:42Z

components/PluginsContainer.jsx

+    isAppMenuCompact = (pluginsConfig)=> {
+        const pluginTopBar = pluginsConfig.find((pluginConf) => pluginConf.name === "TopBar");
+        return pluginTopBar && pluginTopBar.cfg.appMenuCompact;
+    };


Rather than having this logic in the plugin container, I'd introduce an action setRightMargin or similar in actions/map, which sets state.map.rightMargin, and is then honoured by the various components for positioning (Buttons, WindowManager, MapCopyright). The problem with this approach is that, besides not taking mobile mode into account, it will break right-docked windows with splitscreen=true.

Hello @manisandro, I think it is a better way to add an setRightMargin action.
Thanks for your advice.

I'm sorry but I think I didn't manage to set splitscreen=true. How/where to do that ? Or wich behavior in the interface ?

I only have right-docked windows.

Have a good day

Hi @gwenandres , the ResizeableWindow component has a splitScreenWhenDocked prop. But the actual plugin rendering the window needs to set it, if it is useful for the component (i.e. AttributeTable, HeightProfile set splitScreenWhenDocked). For testing, you can just set it for a plugin which renders a right-dockable window.

manisandro · 2024-02-21T17:32:11Z

components/AppMenu.jsx

+            } else  {
+                document.removeEventListener('click', this.checkCloseMenu);
+                document.removeEventListener('keydown', this.onKeyPress, true);
+                document.removeEventListener('mousemove', this.onMouseMove, true);


I just debugged an issue related to these event listeners. If they remain active, they will break arrow-pan of the map and i.e. enter-submit of input fields. So it is undesirable to have these event listeners active unless there really is an open menu.

Ok I will remove these changes

manisandro · 2024-02-22T13:46:01Z

actions/windows.js

    return {
        type: SET_SPLIT_SCREEN,
        windowId: windowId,
        side: side,
-        size: size
+        size: size,
+        menuMargins: menuMargins


I don't think there is any advantage in passing the menuMargins here, since it is identical to the state value. Just use menuMargins in the reducer. You can move the const mapMargins = {...} block in the reducer to a separate funtion which you call both as a result of SET_SPLIT_SCREEN and SET_MENU_MARGIN.

I'm sorry I don't understand how to move the const mapMargins = {...} block to a separate function. mapMargin Values in SET_SPLIT_SCREEN and SET_MENU_MARGIN are different...

manisandro · 2024-02-22T13:51:01Z

components/AppMenu.jsx

+        } else {
+            document.addEventListener('click', this.checkCloseMenu);
+            document.addEventListener('keydown', this.onKeyPress, true);
+            document.addEventListener('mousemove', this.onMouseMove, true);
        }


Please enclose the entire block between 174 and 181 in

if (!this.props.keepMenuOpen) { ... }

manisandro · 2024-02-22T13:52:07Z

components/AppMenu.jsx

@@ -302,6 +305,12 @@ class AppMenu extends React.Component {
            mousetrap(el).bind(this.props.appMenuShortcut, this.toggleMenu);
        }
    };
+    storeRigthMargin = (el) => {


Rigth -> Right

manisandro · 2024-02-22T13:52:35Z

components/AppMenu.jsx

@@ -302,6 +305,12 @@ class AppMenu extends React.Component {
            mousetrap(el).bind(this.props.appMenuShortcut, this.toggleMenu);
        }
    };
+    storeRigthMargin = (el) => {
+        if (this.props.menuCompact && el?.clientWidth > 0) {
+            const rightmargin = el.clientWidth - MiscUtils.convertEmToPx(11.5);


What is 11.5em?

Hello,
11.5em is the right margin used in CSS to hide a part of menu :
https://github.com/qgis/qwc2/pull/308/files#diff-9af8fe3b349e7d4c7e12d7406c0dc4898315de424adb8a5410a783c774ac807bR60

manisandro · 2024-02-22T13:56:15Z

components/ResizeableWindow.jsx

-        const style = {display: this.props.visible ? 'initial' : 'none'};
+        const style = {
+            display: this.props.visible ? 'initial' : 'none',
+            right: 'calc(0.25em + ' + this.props.menuMargins.right + 'px)'


What is 0.25em? Shouldn't this be part of the menuMargin, if necessary? Else with a zero menuMargin, it will still have a 0.25em right offset.
You'll also want to honour the left margin, since you support it in the reducer state.

I add 0.25em as it is done for MapCopyright :

https://github.com/qgis/qwc2/blob/master/plugins/MapCopyright.jsx#L57

I found it aesthetically pleasing when everything wasn't aligned (stuck) with the right edge; even without compact menu.

I hope to honor the left margin but I can't do this right now. Should I remove left margin from reducer state ?

manisandro · 2024-02-22T13:56:37Z

components/SideBar.jsx

@@ -80,6 +81,7 @@ class SideBar extends React.Component {
        const render = visible || this.state.render || this.props.renderWhenHidden;
        const style = {
            width: this.props.width,
+            right: visible ? 'calc(0.25em + ' + this.props.menuMargins.right + 'px)' : 0,


Same question regarding 0.25em as above

Also note that the SideBar can be left-aligned, so you also want honour that case.

manisandro · 2024-02-22T13:58:45Z

reducers/map.js

@@ -27,6 +27,7 @@ const defaultState = {
    resolutions: [0],
    topbarHeight: 0,
    bottombarHeight: 0,
+    rightMargin: 0,


This looks like a leftover

manisandro · 2024-02-22T13:59:13Z

reducers/windows.js

+            bottom: 0,
+            left: action.left,
+            top: 0
+        };


This looks wrong - why reset the top and bottom map margin to zero?

Yes, I correct that

manisandro · 2024-02-28T15:28:53Z

I've merged this manually and added a bunch of fixes, please check out if the latest master works for you. Thanks!

manisandro reviewed Feb 8, 2024

View reviewed changes

gwenandres added 10 commits February 20, 2024 16:58

Add attribute and classes for menuCompact

f21974b

CSS

f861972

remove useless change

a17ed1a

Restrict Menu compact to desktop

348c5a2

Add doc

eabb2d0

Revert use of CSS class to enforce margin right

3318917

Create setRightMargin action

8acbba2

Use setRightMargin action

743553c

Add right-docked splitscreen support for LoginUser plugin

bc27264

Changes after rebase

f75338e

gwenandres force-pushed the ui/menu_compact branch from 97b0e1a to f75338e Compare February 21, 2024 17:14

manisandro reviewed Feb 21, 2024

View reviewed changes

manisandro force-pushed the master branch from 8f5be4d to 814d747 Compare February 21, 2024 22:30

Remove Event listener with menu kept opened

7144161

manisandro reviewed Feb 22, 2024

View reviewed changes

Review

68b103c

manisandro closed this Feb 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

An other way to open menu #308

An other way to open menu #308

gwenandres commented Feb 7, 2024

manisandro commented Feb 8, 2024

manisandro Feb 8, 2024

gwenandres Feb 20, 2024

manisandro Feb 20, 2024

manisandro Feb 21, 2024

gwenandres Feb 22, 2024

manisandro Feb 22, 2024

gwenandres Feb 27, 2024

manisandro Feb 22, 2024

manisandro Feb 22, 2024

manisandro Feb 22, 2024

gwenandres Feb 27, 2024

manisandro Feb 22, 2024

gwenandres Feb 27, 2024

manisandro Feb 22, 2024 •

edited

Loading

manisandro Feb 22, 2024

manisandro Feb 22, 2024

gwenandres Feb 27, 2024

manisandro commented Feb 28, 2024

An other way to open menu #308

An other way to open menu #308

Conversation

gwenandres commented Feb 7, 2024

manisandro commented Feb 8, 2024

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

Choose a reason for hiding this comment

manisandro Feb 22, 2024 • edited Loading

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

manisandro commented Feb 28, 2024

manisandro Feb 22, 2024 •

edited

Loading