-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Changes from all commits
f21974b
f861972
a17ed1a
348c5a2
eabb2d0
3318917
8acbba2
743553c
bc27264
f75338e
7144161
68b103c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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 | ||
}; | ||
|
@@ -168,14 +171,16 @@ 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 { | ||
document.removeEventListener('click', this.checkCloseMenu); | ||
document.removeEventListener('keydown', this.onKeyPress, true); | ||
document.removeEventListener('mousemove', this.onMouseMove, true); | ||
if (!this.props.keepMenuOpen) { | ||
if (!this.state.menuVisible) { | ||
document.addEventListener('click', this.checkCloseMenu); | ||
document.addEventListener('keydown', this.onKeyPress, true); | ||
document.addEventListener('mousemove', this.onMouseMove, true); | ||
} else { | ||
document.removeEventListener('click', this.checkCloseMenu); | ||
document.removeEventListener('keydown', this.onKeyPress, true); | ||
document.removeEventListener('mousemove', this.onMouseMove, true); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please enclose the entire block between 174 and 181 in
|
||
this.props.onMenuToggled(!this.state.menuVisible); | ||
this.setState((state) => ({menuVisible: !state.menuVisible, submenusVisible: [], filter: ""})); | ||
|
@@ -262,20 +267,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.storeRightMargin}> | ||
<ul className="appmenu-menu"> | ||
{this.props.showFilterField ? ( | ||
<li className="appmenu-leaf"> | ||
|
@@ -302,6 +307,12 @@ class AppMenu extends React.Component { | |
mousetrap(el).bind(this.props.appMenuShortcut, this.toggleMenu); | ||
} | ||
}; | ||
storeRightMargin = (el) => { | ||
if (this.props.menuCompact && el?.clientWidth > 0) { | ||
const rightmargin = el.clientWidth - MiscUtils.convertEmToPx(11.5); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is 11.5em? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello, |
||
this.props.setMenuMargin(rightmargin, 0); | ||
} | ||
}; | ||
itemAllowed = (item) => { | ||
if (!ThemeUtils.themFlagsAllowed(this.props.currentTheme, item.themeFlagWhitelist, item. themeFlagBlacklist)) { | ||
return false; | ||
|
@@ -323,5 +334,6 @@ export default connect((state) => ({ | |
currentTaskBlocked: state.task.blocked, | ||
currentTheme: state.theme.current || {} | ||
}), { | ||
setCurrentTask: setCurrentTask | ||
setCurrentTask: setCurrentTask, | ||
setMenuMargin: setMenuMargin | ||
})(AppMenu); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -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); | ||
} | ||
} | ||
componentDidUpdate(prevProps, prevState) { | ||
|
@@ -135,7 +136,7 @@ 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); | ||
} 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; | ||
|
@@ -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)' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
|
@@ -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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
}; | ||
|
@@ -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, { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,8 @@ import { | |
REGISTER_WINDOW, | ||
UNREGISTER_WINDOW, | ||
RAISE_WINDOW, | ||
SET_SPLIT_SCREEN | ||
SET_SPLIT_SCREEN, | ||
SET_MENU_MARGIN | ||
} from '../actions/windows'; | ||
|
||
const defaultState = { | ||
|
@@ -23,6 +24,9 @@ const defaultState = { | |
mapMargins: { | ||
left: 0, top: 0, right: 0, bottom: 0 | ||
}, | ||
menuMargins: { | ||
left: 0, right: 0 | ||
}, | ||
entries: {} | ||
}; | ||
|
||
|
@@ -95,9 +99,9 @@ export default function windows(state = defaultState, action) { | |
} | ||
const splitWindows = Object.values(newSplitScreen); | ||
const mapMargins = { | ||
right: splitWindows.filter(entry => entry.side === 'right').reduce((res, e) => Math.max(e.size, res), 0), | ||
right: splitWindows.filter(entry => entry.side === 'right').reduce((res, e) => Math.max(e.size, res), 0) + state.menuMargins.right, | ||
bottom: splitWindows.filter(entry => entry.side === 'bottom').reduce((res, e) => Math.max(e.size, res), 0), | ||
left: splitWindows.filter(entry => entry.side === 'left').reduce((res, e) => Math.max(e.size, res), 0), | ||
left: splitWindows.filter(entry => entry.side === 'left').reduce((res, e) => Math.max(e.size, res), 0) + state.menuMargins.left, | ||
top: splitWindows.filter(entry => entry.side === 'top').reduce((res, e) => Math.max(e.size, res), 0) | ||
}; | ||
return { | ||
|
@@ -106,6 +110,19 @@ export default function windows(state = defaultState, action) { | |
mapMargins: mapMargins | ||
}; | ||
} | ||
case SET_MENU_MARGIN: { | ||
const menuMargins = { | ||
right: action.right, | ||
left: action.left | ||
}; | ||
const mapMargins = { | ||
right: state.mapMargins.right + action.right, | ||
bottom: state.mapMargins.bottom, | ||
left: state.mapMargins.left + action.left, | ||
top: state.mapMargins.top | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks wrong - why reset the top and bottom map margin to zero? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I correct that |
||
return {...state, menuMargins: menuMargins, mapMargins: mapMargins}; | ||
} | ||
default: | ||
return state; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I will remove these changes