Skip to content

Closes #2203. GetFeatureInfo active by default#2427

Merged
offtherailz merged 2 commits intogeosolutions-it:masterfrom
kappu72:i#2203
Dec 4, 2017
Merged

Closes #2203. GetFeatureInfo active by default#2427
offtherailz merged 2 commits intogeosolutions-it:masterfrom
kappu72:i#2203

Conversation

@kappu72
Copy link
Copy Markdown
Contributor

@kappu72 kappu72 commented Nov 23, 2017

Description

The GFI tool has become active by default. The user just needs to click on map to get the feature info. The GFI is stopped if Annotation or Grid plugins are editing, draw or measure support are active.

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
The GFI is enabled from the btn in toolbar. It's disabled by some other tolls but not reenabled.

What is the new behavior?
The GFI is enabled by default, and disabled/enabled under some circumstance.

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes
  • No

Other information:
It is possible to control the GFI, by setting the state variables: enabled and disableAlwaysOn.

@ghost ghost assigned kappu72 Nov 23, 2017
@ghost ghost added In Test labels Nov 23, 2017
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 81.085% when pulling bb8ad30 on kappu72:i#2203 into 98d7440 on geosolutions-it:master.

}),
changeMapPointer: (action$, store) =>
action$.ofType(CHANGE_MOUSE_POINTER)
.filter(() => !(store.getState()).map)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Everytime you access the state, it is preferrable to use a selector

.switchMap((a) => action$.ofType(MAP_CONFIG_LOADED).mapTo(a)),
onMapClick: (action$, store) =>
action$.ofType(CLICK_ON_MAP).filter(() => {
const {disableAlwaysOn = false} = (store.getState()).mapInfo;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add or use existing selector

Copy link
Copy Markdown
Contributor Author

@kappu72 kappu72 Nov 24, 2017

Choose a reason for hiding this comment

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

Ok, but here we are not computing any derivate value, so what's the meaning of using a selector?

const drawStatus = get(state, "draw.drawStatus", false);
return drawStatus && drawStatus !== 'clean' && drawStatus !== 'stop';
};
const gridEditingSelector = (state) => get(state, "featuregrid.mode") === 'EDIT';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the feature grid mode selector already exists, please use that as part of this selector

Copy link
Copy Markdown
Contributor Author

@kappu72 kappu72 Nov 24, 2017

Choose a reason for hiding this comment

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

It's not exactly the same selector one select the mode, this select the editing mode, anyway If you prefer I'll change

.switchMap(() => {
return Rx.Observable.of(closeFeatureGrid());
}),
changeMapPointer: (action$, store) =>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure about this epic, can you explain it?

Copy link
Copy Markdown
Contributor Author

@kappu72 kappu72 Nov 24, 2017

Choose a reason for hiding this comment

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

When the GFI is enabled by default, It could throw the change-map-pointer action before the map is configured so the mouse pointer action will fail

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe add a comment above this epic with the answer you give me. :)

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 81.086% when pulling eeaec6a on kappu72:i#2203 into 98d7440 on geosolutions-it:master.

@offtherailz offtherailz changed the title Closes #2203 Closes #2203. GetFeatureInfo active by default Nov 30, 2017
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 81.019% when pulling b584eb3 on kappu72:i#2203 into ac62205 on geosolutions-it:master.

@offtherailz offtherailz merged commit 4c2737b into geosolutions-it:master Dec 4, 2017
@ghost ghost added Accepted and removed pending review labels Dec 4, 2017
offtherailz added a commit that referenced this pull request Dec 4, 2017
@tdipisa tdipisa removed the Accepted label Jan 8, 2018
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.

Always-on GetFeatureInfo

5 participants