-
Notifications
You must be signed in to change notification settings - Fork 400
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
Fix #1398 Improve query panel behaviour #1413
Fix #1398 Improve query panel behaviour #1413
Conversation
* close drawer menu on search * close query results on back to search * close query results on toggle drawer control * limit the movement of height of query results dock
this.props.onClose(); | ||
}, | ||
limitDockHeight(size) { | ||
const minSize = 0.25; |
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.
This should be configurable using a prop
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.
should be configurable as a prop
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.
Should they be also action and reducer or only props of the class? (minSize and maxSize)
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.
props of the component and of the plugin (to be customizable in localConfig cfg entry). Pass them from the FeatureGrid Plugin down to this component.
web/client/plugins/DrawerMenu.jsx
Outdated
@@ -15,6 +15,8 @@ const {toggleControl, setControlProperty} = require('../actions/controls'); | |||
|
|||
const {changeMapStyle} = require('../actions/map'); | |||
|
|||
const {featureClose} = require('../actions/wfsquery'); |
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.
We are putting querybuilder / featuregrid logic inside a generic container: it's not the right place.
We already have knowledge of querybuilder in the TOC. I don't want to mix stuff too much.
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.
Maybe the right place to trigger this action is inside the toggleMenu actionCreator, using thunk to dispatch both actions.
web/client/translations/data.fr-FR
Outdated
@@ -693,6 +693,7 @@ | |||
"export": "Exporter", | |||
"selectall": "Tout sélectionner", | |||
"deselectall": "Tout désélectionner", | |||
"backtosearch": "Back to search", |
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.
Retour à la recherche
web/client/translations/data.de-DE
Outdated
@@ -691,6 +691,7 @@ | |||
"export": "Export", | |||
"selectall": "Alle auswählen", | |||
"deselectall": "Auswahl aufheben", | |||
"backtosearch": "Back to search", |
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.
Zurück zur Suche (?)
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.
Maybe the right place to trigger this action is inside the toggleMenu actionCreator, using thunk to dispatch both actions.
this.props.onClose(); | ||
}, | ||
limitDockHeight(size) { | ||
const minSize = 0.25; |
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.
should be configurable as a prop
}, | ||
limitDockHeight(size) { | ||
const minSize = 0.25; | ||
const maxSize = 0.85; |
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.
Should be configurable, 1 by default
* Set max and min height of dock query in props/localConfig * Add disabled control on drawer menu * Update FR and DE translation
web/client/actions/wfsquery.js
Outdated
@@ -15,6 +15,7 @@ const QUERY_CREATE = 'QUERY_CREATE'; | |||
const QUERY_RESULT = 'QUERY_RESULT'; | |||
const QUERY_ERROR = 'QUERY_ERROR'; | |||
const RESET_QUERY = 'RESET_QUERY'; | |||
const QUERY_DOCK_SIZE = 'QUERY_DOCK_SIZE'; |
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.
Should we move this to another file? actions/featuregrid?
web/client/actions/wfsquery.js
Outdated
@@ -74,6 +75,13 @@ function queryError(error) { | |||
}; | |||
} | |||
|
|||
function queryDockSize(dockSize) { |
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.
Should we move this to another file? actions/featuregrid?
@@ -54,7 +53,13 @@ const DockedFeatureGrid = React.createClass({ | |||
cleanError: React.PropTypes.func, | |||
selectAllToggle: React.PropTypes.func, | |||
zoomToFeatureAction: React.PropTypes.func, | |||
onClose: React.PropTypes.func | |||
onClose: React.PropTypes.func, | |||
openDrawer: React.PropTypes.func, |
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.
onBackToSearch, to replace both openDrawer and enableDrawer: the featuregrid is not aware of drawer existance, and should not be. Each component is independent. We can glue things together in the plugins.
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.
Should we include also onClose in onBackToSearch?
function closeResponse() {
return (dispatch, getState) => {
dispatch(featureClose()); // old onClose
if (getState().controls && getState().controls.drawer) {
dispatch(setControlProperty('drawer', 'enabled', true)); // old openDrawer
dispatch(setControlProperty('drawer', 'disabled', false)); // old enableDrawer
}
};
}
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.
Yes, I think it's better
@@ -295,6 +303,20 @@ const DockedFeatureGrid = React.createClass({ | |||
selectFeatures(features) { | |||
this.props.selectAllToggle(); | |||
this.props.selectFeatures(features); | |||
}, | |||
backToSearch() { | |||
this.props.openDrawer(); |
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.
this.onBackToSearch()
@@ -103,6 +105,8 @@ const QueryToolbar = React.createClass({ | |||
sortOptions: this.props.sortOptions, | |||
hits: this.props.hits | |||
}; | |||
this.props.actions.closeDrawer(); |
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.
closeDrawer and disableDrawer should be handled in onQuery. QueryToolbar does not know it's included in a drawer
web/client/reducers/query.js
Outdated
@@ -16,7 +16,8 @@ const { | |||
QUERY_RESULT, | |||
QUERY_ERROR, | |||
RESET_QUERY, | |||
FEATURE_CLOSE | |||
FEATURE_CLOSE, | |||
QUERY_DOCK_SIZE |
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.
reducers/featuregrid.js?
web/client/reducers/query.js
Outdated
@@ -148,6 +150,11 @@ function query(state = initialState, action) { | |||
open: false | |||
}); | |||
} | |||
case QUERY_DOCK_SIZE: { |
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.
reducers/featuregrid.js?
@@ -95,7 +95,8 @@ const QueryBuilder = React.createClass({ | |||
onQuery: () => {}, | |||
onReset: () => {}, | |||
onChangeDrawingStatus: () => {}, | |||
onToggleDrawer: () => {} | |||
closeDrawer: () => {}, | |||
disableDrawer: () => {} |
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.
This functions be called into the onQuery action, if needed (using thunk)
@@ -104,7 +105,8 @@ const QueryToolbar = React.createClass({ | |||
sortOptions: this.props.sortOptions, | |||
hits: this.props.hits | |||
}; | |||
this.props.actions.onToggleDrawer(); |
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.
same here, there should be an thunk that opens the bottom table and closes the left menu.
Better if it happens when the search result arrives.
web/client/plugins/TOC.jsx
Outdated
@@ -117,7 +117,8 @@ const SmartQueryForm = connect((state) => { | |||
onQuery: createQuery, | |||
onReset: reset, | |||
onChangeDrawingStatus: changeDrawingStatus, | |||
onToggleDrawer: toggleControl.bind(null, 'drawer', null) | |||
closeDrawer: setControlProperty.bind(null, 'drawer', 'enabled', false), |
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.
for the reasons explained above of course these functions should not be passed here.
The reason why you disable the drawer menu button is that this way you avoid the user to do things with the layer tree that may make back to search button meanful? |
Yes, it's for that reason. I think we should give only one way to the user to come back to the layer tree. |
preview open/close panels:
dock minimum and maximum height positions: