-
Notifications
You must be signed in to change notification settings - Fork 416
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
#6758 Fixed default custom editors and FeatureEditor config in context #6836
Conversation
…onfig in context
@tdipisa didn't put the milestone to this PR (should it be backported, so having a new minor, or not)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few comments
the PR is good other than that
web/client/plugins/FeatureEditor.jsx
Outdated
componentDidUpdate(oldProps) { | ||
const newOptions = pick(this.props, ['showFilteredObject', 'showTimeSync', 'timeSync', 'customEditorsOptions']); | ||
const oldOptions = pick(oldProps, ['showFilteredObject', 'showTimeSync', 'timeSync', 'customEditorsOptions']); | ||
if (JSON.stringify(newOptions) !== JSON.stringify(oldOptions) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why stringify and not !isEqual or deepEqual from lodash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll replace it with a deepEqual maybe be more robust and predictable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok :)
Draft for tests that have to be refined/checked
Description
This PR solves the 2 points in this comment #6758 (comment)
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
What is the current behavior?
#6758
What is the new behavior?
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information
For testing:
The problems that caused this PR are 2:
customEditorsOptions
) in contexts (they work in localConfig)So to test this you can create a context with the
DropDownEditor
andNumberEditor
configured and check (example on dev ).Trying to open the feature grid in edit mode (logged in as admin) you should be able to
type
ONLY with the values "FOREST", "MOUNTAIN", "DESERT", "LAKE", "SECRET" without possibility to keep it empty or any pagination in the dropdown.land
attribute can not be a negative numberfor a complete information and to replicate in other contexts.
The map in the example context contains the layer
atlantis:landmarks
and has this configuration of Attribute table: