Skip to content

Externalized in epic getFeatureInfo retrieval logic#3616

Merged
offtherailz merged 1 commit intogeosolutions-it:masterfrom
offtherailz:externalize_feature_info_request
Mar 13, 2019
Merged

Externalized in epic getFeatureInfo retrieval logic#3616
offtherailz merged 1 commit intogeosolutions-it:masterfrom
offtherailz:externalize_feature_info_request

Conversation

@offtherailz
Copy link
Copy Markdown
Member

@offtherailz offtherailz commented Mar 13, 2019

Description

This pull request moves the request build and trigger logic from the FeatureInfoContainer to the identify epic. This

  • simplify the logic following SRP
  • Allow to intercept and modify the request sequence in an efficient and simple way.

This refactor is required to allow clicked feature highlight (#3617)

Please check if the PR fulfills these requirements

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

  • Refactoring (no functional changes, no api changes)

What is the current behavior? (You can also link to an open issue here)
No changes

What is the new behavior?
Same as before. The only difference is about multiSelection (disabled by default:

  • The multiSelection flag is has been moved temporary from the Component configuration onto the action (is not configurable, we need a little more work to make configurable from the app state, but I didn't want to complicate things for c125_annotations merge too much, for a feature that is not used neither documented).

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

  • No

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.05%) to 80.888% when pulling 0c2e58c on offtherailz:externalize_feature_info_request into cd62de6 on geosolutions-it:master.

@offtherailz offtherailz added this to the 2019.02.00 milestone Mar 13, 2019
headerGlyph: "",
closeGlyph: "1-close",
className: "square-button",
allowMultiselection: false,
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.

I would not remove this without an explicit reason.

Copy link
Copy Markdown
Member Author

@offtherailz offtherailz Mar 13, 2019

Choose a reason for hiding this comment

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

I partially maintained it (that it means that is not configurable anymore, but the code base for this flag is still maintained and tested).

Ideally, after this changes, it should go in the application state.
If you want I can finalize the support for this flag in the mapInfo state.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@offtherailz offtherailz requested a review from mbarto March 13, 2019 13:51
@offtherailz offtherailz merged commit e68ce42 into geosolutions-it:master Mar 13, 2019
@offtherailz offtherailz deleted the externalize_feature_info_request branch May 26, 2020 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants