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

Fix #1398 Improve query panel behaviour #1413

Merged
merged 3 commits into from
Feb 7, 2017

Conversation

allyoucanmap
Copy link
Contributor

  • 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

preview open/close panels:
querypanel

dock minimum and maximum height positions:
minmaxheight

* 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
@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 80.43% when pulling 031c1ad on allyoucanmap:improve-query-panel into d70b92b on geosolutions-it:master.

this.props.onClose();
},
limitDockHeight(size) {
const minSize = 0.25;
Copy link
Contributor

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

Copy link
Member

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

Copy link
Contributor Author

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)

Copy link
Member

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.

@@ -15,6 +15,8 @@ const {toggleControl, setControlProperty} = require('../actions/controls');

const {changeMapStyle} = require('../actions/map');

const {featureClose} = require('../actions/wfsquery');
Copy link
Contributor

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.

Copy link
Member

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.

@@ -693,6 +693,7 @@
"export": "Exporter",
"selectall": "Tout sélectionner",
"deselectall": "Tout désélectionner",
"backtosearch": "Back to search",
Copy link
Member

Choose a reason for hiding this comment

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

Retour à la recherche

@@ -691,6 +691,7 @@
"export": "Export",
"selectall": "Alle auswählen",
"deselectall": "Auswahl aufheben",
"backtosearch": "Back to search",
Copy link
Member

Choose a reason for hiding this comment

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

Zurück zur Suche (?)

Copy link
Member

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;
Copy link
Member

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;
Copy link
Member

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
@allyoucanmap
Copy link
Contributor Author

allyoucanmap commented Jan 31, 2017

in the previous commit we could back to the TOC menu in two ways from 'back to search' button or
the drawer button but this logic caused problems in DrawerMenu.jsx

before

now we have only one way to back to search menu with the query results 'back to search' button

now

preview new dock limit
newminmaxheight

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.8%) to 76.608% when pulling abcd66e on allyoucanmap:improve-query-panel into d70b92b on geosolutions-it:master.

@@ -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';
Copy link
Contributor

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?

@@ -74,6 +75,13 @@ function queryError(error) {
};
}

function queryDockSize(dockSize) {
Copy link
Contributor

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,
Copy link
Contributor

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.

Copy link
Contributor Author

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
        }
    };
}

Copy link
Contributor

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();
Copy link
Contributor

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();
Copy link
Contributor

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

@@ -16,7 +16,8 @@ const {
QUERY_RESULT,
QUERY_ERROR,
RESET_QUERY,
FEATURE_CLOSE
FEATURE_CLOSE,
QUERY_DOCK_SIZE
Copy link
Contributor

Choose a reason for hiding this comment

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

reducers/featuregrid.js?

@@ -148,6 +150,11 @@ function query(state = initialState, action) {
open: false
});
}
case QUERY_DOCK_SIZE: {
Copy link
Contributor

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: () => {}
Copy link
Member

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();
Copy link
Member

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.

@@ -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),
Copy link
Member

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.

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.8%) to 76.612% when pulling 5d2fe4b on allyoucanmap:improve-query-panel into d70b92b on geosolutions-it:master.

@offtherailz
Copy link
Member

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?

@allyoucanmap
Copy link
Contributor Author

Yes, it's for that reason. I think we should give only one way to the user to come back to the layer tree.
Maybe should we make invisible the drawer button?

@mbarto mbarto merged commit d39d13b into geosolutions-it:master Feb 7, 2017
@allyoucanmap allyoucanmap deleted the improve-query-panel branch May 19, 2017 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants