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

Externalized in epic getFeatureInfo retrieval logic #3616

Conversation

offtherailz
Copy link
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

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
panelClassName: "modal-dialog info-panel modal-content",
headerClassName: "modal-header",
bodyClassName: "modal-body info-wrap",
dock: true,
headerGlyph: "",
closeGlyph: "1-close",
className: "square-button",
allowMultiselection: false,
Copy link
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
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
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.

4 participants