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

#6078: Box Selection on Map #6145

Merged
merged 13 commits into from
Nov 9, 2020
Merged

#6078: Box Selection on Map #6145

merged 13 commits into from
Nov 9, 2020

Conversation

AGMETEOR
Copy link
Contributor

@AGMETEOR AGMETEOR commented Nov 2, 2020

Description

This allows adds box selection interaction to OL/Leaflet maps when featuregrid opens

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: Enhancement

Issue

What is the current behavior?

#6078

What is the new behavior?

Allow box selection on map when geometry filter is enabled in FeatureEditor plugin

Breaking change

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

  • Yes, and I documented them in migration notes
  • No

Other useful information

@coveralls
Copy link

coveralls commented Nov 2, 2020

Coverage Status

Coverage decreased (-0.07%) to 82.652% when pulling 3bd40ad on AGMETEOR:6078_2 into 055bca9 on geosolutions-it:master.

@AGMETEOR AGMETEOR marked this pull request as ready for review November 5, 2020 11:31
@tdipisa tdipisa requested a review from MV88 November 5, 2020 15:46
Copy link
Contributor

@MV88 MV88 left a comment

Choose a reason for hiding this comment

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

  • BUG both maptypes: the selection is no longer active if I enter query panel and exit it with no search (left arrow).

  • BUG only when CTRL is active it will add features, if CTRL is not active only the new selected features will be highlighted

  • BUG If CTRL is active and you click a selected feature it will deselect it as it is now, but it has a strange style
    image

  • BUG leaflet only, multiple selection does not work there, and box selection support seems active from the moment i open the feature grid. check this out
    multiple box selection - leaflet

@tdipisa i also noticed that the features filtered by Box Selection goes in AND with the filter (i.e spatial filter) made in the Query Panel
multiple box selection 2
i think this is expected

@offtherailz @mbarto @allyoucanmap should this pr be refactorized once we have merged the locale refactor ?
since he's introducing a new BoxSelectionSupport, i was wondering if we need more work later on. just saying

@tdipisa
Copy link
Member

tdipisa commented Nov 5, 2020

@AGMETEOR

BUG leaflet only, multiple selection does not work there, and box selection support seems active from the moment i open the feature grid. check this out

We don't strictly need this support available for Leaflet now. Therefore is, sice we are in hurry, the Leaflet support for this can be removed if it requires too much time to be fixed.

@MV88

@tdipisa i also noticed that the features filtered by Box Selection goes in AND with the filter (i.e spatial filter) made in the Query Panel. I think this is expected

Yes, also the selection on click is working in the same way now in DEV, therefore it is consistent.

@AGMETEOR AGMETEOR requested a review from MV88 November 6, 2020 13:10
Copy link
Contributor

@MV88 MV88 left a comment

Choose a reason for hiding this comment

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

  • BUG

  • select with box selection some non highlighted features two times

  • remove some highlighted one with ctrl click

  • select some features that are partially selected and partially not, by ending on an highlighted one

  • open query panel and

  • you get this error with gray page (app crash)

webpack:///./web/client/components/app/StandardRouter.jsx?:135 TypeError: Cannot read property 'forEach' of undefined
    at VectorSource.removeFeatureInternal (webpack:///./node_modules/ol/source/Vector.js?:971)
    at VectorSource.removeFeature (webpack:///./node_modules/ol/source/Vector.js?:959)
    at eval (webpack:///./web/client/components/map/openlayers/Feature.jsx?:169)
    at Array.map (<anonymous>)
    at Feature.eval [as removeFromContainer] (webpack:///./web/client/components/map/openlayers/Feature.jsx?:165)
    at Feature.componentWillUnmount (webpack:///./web/client/components/map/openlayers/Feature.jsx?:202)
    at callComponentWillUnmountWithTimer (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:21757)
    at HTMLUnknownElement.callCallback (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:362)
    at Object.invokeGuardedCallbackDev (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:411)
    at invokeGuardedCallback (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:466)
    at safelyCallComponentWillUnmount (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:21764)
    at commitUnmount (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:22257)
    at unmountHostComponents (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:22733)
    at commitDeletion (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:22769)
    at commitMutationEffects (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:25267)
    at HTMLUnknownElement.callCallback (webpack:///./node_modules/react-dom/cjs/react-dom.development.js?:362)
  • BUG Also in a normal situation (when you just select some features with box selection) you cannot use box selection when you enter and exit query panel even if the filter is active, this was already notified in the previous review
    expected result is to be able to add more features with CTRL once the query panel is closed with the arrow

@AGMETEOR AGMETEOR requested a review from MV88 November 6, 2020 15:56
Copy link
Contributor

@MV88 MV88 left a comment

Choose a reason for hiding this comment

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

click does not work when the feature grid reopens after going back from the advanced search panel (query panel)

@tdipisa
since this exists on master, shall we move on with this feature and fix this in a separate issue?

@tdipisa
Copy link
Member

tdipisa commented Nov 9, 2020

click does not work when the feature grid reopens after going back from the advanced search panel (query panel)

@tdipisa
since this exists on master, shall we move on with this feature and fix this in a separate issue?

Ok @MV88 please open the issue and assign it to @AGMETEOR. Beside that, can we merge this PR?

@MV88
Copy link
Contributor

MV88 commented Nov 9, 2020

click does not work when the feature grid reopens after going back from the advanced search panel (query panel)
@tdipisa
since this exists on master, shall we move on with this feature and fix this in a separate issue?

Ok @MV88 please open the issue and assign it to @AGMETEOR. Beside that, can we merge this PR?

i think so, because the latest points notified seems working now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants