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

An other way to open menu #308

Closed
wants to merge 12 commits into from
14 changes: 12 additions & 2 deletions actions/windows.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const CLOSE_ALL_WINDOWS = 'CLOSE_ALL_WINDOWS';
export const REGISTER_WINDOW = 'REGISTER_WINDOW';
export const UNREGISTER_WINDOW = 'UNREGISTER_WINDOW';
export const RAISE_WINDOW = 'RAISE_WINDOW';
export const SET_MENU_MARGIN = 'SET_MENU_MARGIN';
export const SET_SPLIT_SCREEN = 'SET_SPLIT_SCREEN';

export const NotificationType = {
Expand Down Expand Up @@ -78,11 +79,20 @@ export function raiseWindow(id) {
};
}

export function setSplitScreen(windowId, side, size) {
export function setSplitScreen(windowId, side, size, menuMargins) {
return {
type: SET_SPLIT_SCREEN,
windowId: windowId,
side: side,
size: size
size: size,
menuMargins: menuMargins
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

};
}

export function setMenuMargin(right, left) {
return {
type: SET_MENU_MARGIN,
right: right,
left: left
};
}
36 changes: 23 additions & 13 deletions components/AppMenu.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import mousetrap from 'mousetrap';
import {remove as removeDiacritics} from 'diacritics';
import isEmpty from 'lodash.isempty';
import classnames from 'classnames';
import {setMenuMargin} from '../actions/windows';
import {setCurrentTask} from '../actions/task';
import InputContainer from '../components/InputContainer';
import LocaleUtils from '../utils/LocaleUtils';
Expand All @@ -32,10 +33,12 @@ class AppMenu extends React.Component {
currentTaskBlocked: PropTypes.bool,
currentTheme: PropTypes.object,
keepMenuOpen: PropTypes.bool,
menuCompact: PropTypes.bool,
menuItems: PropTypes.array,
onMenuToggled: PropTypes.func,
openExternalUrl: PropTypes.func,
setCurrentTask: PropTypes.func,
setMenuMargin: PropTypes.func,
showFilterField: PropTypes.bool,
showOnStartup: PropTypes.bool
};
Expand Down Expand Up @@ -168,14 +171,14 @@ class AppMenu extends React.Component {
if (!this.state.menuVisible && this.props.appMenuClearsTask) {
this.props.setCurrentTask(null);
}
if (!this.state.menuVisible) {
document.addEventListener('click', this.checkCloseMenu);
document.addEventListener('keydown', this.onKeyPress, true);
document.addEventListener('mousemove', this.onMouseMove, true);
} else {
if (this.state.menuVisible || this.props.keepMenuOpen) {
document.removeEventListener('click', this.checkCloseMenu);
document.removeEventListener('keydown', this.onKeyPress, true);
document.removeEventListener('mousemove', this.onMouseMove, true);
} else {
document.addEventListener('click', this.checkCloseMenu);
document.addEventListener('keydown', this.onKeyPress, true);
document.addEventListener('mousemove', this.onMouseMove, true);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please enclose the entire block between 174 and 181 in

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

this.props.onMenuToggled(!this.state.menuVisible);
this.setState((state) => ({menuVisible: !state.menuVisible, submenusVisible: [], filter: ""}));
Expand Down Expand Up @@ -262,20 +265,20 @@ class AppMenu extends React.Component {
}
};
render() {
let className = "";
if (this.props.currentTaskBlocked) {
className = "appmenu-blocked";
} else if (this.state.menuVisible) {
className = "appmenu-visible";
}
const visible = !this.props.currentTaskBlocked && this.state.menuVisible;
const className = classnames({
"appmenu-blocked": this.props.currentTaskBlocked,
"appmenu-visible": visible,
"appmenu-compact": this.props.menuCompact
});
const filter = removeDiacritics(this.state.filter.toLowerCase());
return (
<div className={"AppMenu " + className} ref={el => { this.menuEl = el; MiscUtils.setupKillTouchEvents(el); }}
>
<div className="appmenu-button-container" onMouseDown={this.toggleMenu} >
{this.props.buttonContents}
</div>
<div className="appmenu-menu-container">
<div className="appmenu-menu-container" ref={this.storeRigthMargin}>
<ul className="appmenu-menu">
{this.props.showFilterField ? (
<li className="appmenu-leaf">
Expand All @@ -302,6 +305,12 @@ class AppMenu extends React.Component {
mousetrap(el).bind(this.props.appMenuShortcut, this.toggleMenu);
}
};
storeRigthMargin = (el) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rigth -> Right

if (this.props.menuCompact && el?.clientWidth > 0) {
const rightmargin = el.clientWidth - MiscUtils.convertEmToPx(11.5);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is 11.5em?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.props.setMenuMargin(rightmargin, 0);
}
};
itemAllowed = (item) => {
if (!ThemeUtils.themFlagsAllowed(this.props.currentTheme, item.themeFlagWhitelist, item. themeFlagBlacklist)) {
return false;
Expand All @@ -323,5 +332,6 @@ export default connect((state) => ({
currentTaskBlocked: state.task.blocked,
currentTheme: state.theme.current || {}
}), {
setCurrentTask: setCurrentTask
setCurrentTask: setCurrentTask,
setMenuMargin: setMenuMargin
})(AppMenu);
17 changes: 11 additions & 6 deletions components/ResizeableWindow.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class ResizeableWindow extends React.Component {
maxHeight: PropTypes.number,
maxWidth: PropTypes.number,
maximizeable: PropTypes.bool,
menuMargins: PropTypes.object,
minHeight: PropTypes.number,
minWidth: PropTypes.number,
minimizeable: PropTypes.bool,
Expand Down Expand Up @@ -100,7 +101,7 @@ class ResizeableWindow extends React.Component {
if (props.splitScreenWhenDocked && this.state.geometry.docked) {
const dockSide = props.dockable === true ? "left" : props.dockable;
const dockSize = ["left", "right"].includes(dockSide) ? this.state.geometry.width : this.state.geometry.height;
props.setSplitScreen(this.id, dockSide, dockSize);
props.setSplitScreen(this.id, dockSide, dockSize, this.props.menuMargins);
}
}
computeInitialX = (x) => {
Expand All @@ -117,7 +118,7 @@ class ResizeableWindow extends React.Component {
componentWillUnmount() {
this.props.unregisterWindow(this.id);
if (this.props.splitScreenWhenDocked) {
this.props.setSplitScreen(this.id, null);
this.props.setSplitScreen(this.id, null, null, this.props.menuMargins);
}
}
componentDidUpdate(prevProps, prevState) {
Expand All @@ -135,11 +136,11 @@ class ResizeableWindow extends React.Component {
(!this.props.visible && prevProps.visible) ||
(this.state.geometry.docked === false && prevState.geometry.docked !== false)
) {
this.props.setSplitScreen(this.id, null);
this.props.setSplitScreen(this.id, null, null, this.props.menuMargins);
} else if (this.props.visible && this.state.geometry.docked) {
const dockSide = this.props.dockable === true ? "left" : this.props.dockable;
const dockSize = ["left", "right"].includes(dockSide) ? this.state.geometry.width : this.state.geometry.height;
this.props.setSplitScreen(this.id, dockSide, dockSize);
this.props.setSplitScreen(this.id, dockSide, dockSize, this.props.menuMargins);
}
}
}
Expand Down Expand Up @@ -169,7 +170,10 @@ class ResizeableWindow extends React.Component {
"resizeable-window-body-scrollable": this.props.scrollable,
"resizeable-window-body-nonscrollable": !this.props.scrollable
});
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)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

};
const maximized = this.state.geometry.maximized ? true : false;
const minimized = this.state.geometry.minimized ? true : false;
const zIndex = this.props.baseZIndex + this.props.windowStacking.findIndex(item => item === this.id);
Expand Down Expand Up @@ -327,7 +331,8 @@ class ResizeableWindow extends React.Component {
export default connect((state) => ({
windowStacking: state.windows.stacking,
topbarHeight: state.map.topbarHeight,
bottombarHeight: state.map.bottombarHeight
bottombarHeight: state.map.bottombarHeight,
menuMargins: state.windows.menuMargins
}), {
raiseWindow: raiseWindow,
registerWindow: registerWindow,
Expand Down
5 changes: 4 additions & 1 deletion components/SideBar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class SideBar extends React.Component {
heightResizeable: PropTypes.bool,
icon: PropTypes.string,
id: PropTypes.string.isRequired,
menuMargins: PropTypes.object,
minWidth: PropTypes.string,
onHide: PropTypes.func,
onShow: PropTypes.func,
Expand Down Expand Up @@ -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,
Copy link
Member

@manisandro manisandro Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question regarding 0.25em as above

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

minWidth: this.props.minWidth,
zIndex: visible ? 5 : 4
};
Expand Down Expand Up @@ -166,7 +168,8 @@ class SideBar extends React.Component {
}

const selector = (state) => ({
currentTask: state.task
currentTask: state.task,
menuMargins: state.windows.menuMargins
});

export default connect(selector, {
Expand Down
18 changes: 18 additions & 0 deletions components/style/AppMenu.css
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ div.AppMenu.appmenu-visible {
overflow: visible;
}

div.AppMenu.appmenu-visible.appmenu-compact {
background: none;
}

div.AppMenu .appmenu-label {
font-weight: bold;
transition: color 0.25s;
Expand Down Expand Up @@ -53,6 +57,20 @@ div.AppMenu div.appmenu-menu-container {
max-height: calc(var(--vh, 1vh) * 100 - 5.8em);
}

div.AppMenu.appmenu-compact div.appmenu-menu-container {
right: -11.5em;
width: 15em;
height: calc(100vh - 86px);
transition: transform 0.25s, opacity 0.25s, right 0.5s;
background: var(--app-menu-bg-color);
box-shadow: 0px 0px 4px rgba(136, 136, 136, 0.5);
top: 3.7em;
}

div.AppMenu.appmenu-compact div.appmenu-menu-container:hover {
right: 0;
}

div.AppMenu ul.appmenu-menu {
text-align: left;
padding: 0;
Expand Down
1 change: 1 addition & 0 deletions components/style/ResizeableWindow.css
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ div.resizeable-window-container {
right: 0;
pointer-events: none;
}

div.resizeable-window {
pointer-events: auto;
background-color: var(--container-bg-color);
Expand Down
1 change: 1 addition & 0 deletions doc/plugins.md
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ Top bar, containing the logo, searchbar, task buttons and app menu.
| Property | Type | Description | Default value |
|----------|------|-------------|---------------|
| appMenuClearsTask | `bool` | Whether opening the app menu clears the active task. | `undefined` |
| appMenuCompact | `bool` | Whether show an appMenu compact (menu visible on icons hover) - Only available for desktop client. | `undefined` |
| appMenuFilterField | `bool` | Whether to display the filter field in the app menu. | `undefined` |
| appMenuShortcut | `string` | The shortcut for tiggering the app menu, i.e. alt+shift+m. | `undefined` |
| appMenuVisibleOnStartup | `bool` | Whether to open the app menu on application startup. | `undefined` |
Expand Down
18 changes: 16 additions & 2 deletions plugins/LoginUser.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*/

import React from 'react';
import PropTypes from 'prop-types';
import {connect} from 'react-redux';
import Icon from '../components/Icon';
import ConfigUtils from '../utils/ConfigUtils';
import './style/LoginUser.css';
Expand All @@ -15,17 +17,29 @@ import './style/LoginUser.css';
/**
* Displays the currently logged in user.
*/
export default class LoginUser extends React.Component {
class LoginUser extends React.Component {
static propTypes = {
mapMargins: PropTypes.object
};
render() {
const username = ConfigUtils.getConfigProp("username");
const right = this.props.mapMargins.right;
const style = {
right: 'calc(0.25em + ' + right + 'px)'
};
if (!username) {
return null;
}
return (
<div className="login-user">
<div className="login-user" style={style}>
<Icon icon="login" />
<span>{username}</span>
</div>
);
}
}

export default connect((state) => ({
mapMargins: state.windows.mapMargins
}))(LoginUser);

14 changes: 12 additions & 2 deletions plugins/TopBar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class TopBar extends React.Component {
static propTypes = {
/** Whether opening the app menu clears the active task. */
appMenuClearsTask: PropTypes.bool,
/** Whether show an appMenu compact (menu visible on icons hover) - Only available for desktop client. */
appMenuCompact: PropTypes.bool,
/** Whether to display the filter field in the app menu. */
appMenuFilterField: PropTypes.bool,
/** The shortcut for tiggering the app menu, i.e. alt+shift+m. */
Expand Down Expand Up @@ -99,7 +101,7 @@ class TopBar extends React.Component {
let logo;
const assetsPath = ConfigUtils.getAssetsPath();
const tooltip = LocaleUtils.tr("appmenu.menulabel");
if (this.props.mobile) {
if (this.props.mobile || this.props.appMenuCompact) {
buttonContents = (
<span className="appmenu-button">
<Icon className="appmenu-icon" icon="menu-hamburger" title={tooltip}/>
Expand Down Expand Up @@ -128,6 +130,12 @@ class TopBar extends React.Component {
const searchOptions = {...this.props.searchOptions};
searchOptions.minScaleDenom = searchOptions.minScaleDenom || searchOptions.minScale;
delete searchOptions.minScale;
// Menu compact only available for desktop client
const menuCompact = !this.props.mobile ? this.props.appMenuCompact : false;
// Keep menu open when appMenu is in compact mode (Visible on Hover)
const keepMenuOpen = menuCompact;
// Menu should be visible on startup when appMenu is in compact mode (Visible on Hover)
const showOnStartup = this.props.appMenuVisibleOnStartup || menuCompact;
return (
<Swipeable
onSwipedDown={() => this.props.toggleFullscreen(false)}
Expand All @@ -150,10 +158,12 @@ class TopBar extends React.Component {
appMenuClearsTask={this.props.appMenuClearsTask}
appMenuShortcut={this.props.appMenuShortcut}
buttonContents={buttonContents}
keepMenuOpen={keepMenuOpen}
menuCompact={menuCompact}
menuItems={this.props.menuItems}
openExternalUrl={this.openUrl}
showFilterField={this.props.appMenuFilterField}
showOnStartup={this.props.appMenuVisibleOnStartup} />
showOnStartup={showOnStartup} />
) : null}
{this.props.components.FullscreenSwitcher ? (
<this.props.components.FullscreenSwitcher />
Expand Down
1 change: 1 addition & 0 deletions reducers/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const defaultState = {
resolutions: [0],
topbarHeight: 0,
bottombarHeight: 0,
rightMargin: 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a leftover

click: null,
snapping: {
enabled: false,
Expand Down
Loading
Loading