-
Notifications
You must be signed in to change notification settings - Fork 409
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
#10736: Interactive legend for TOC layers [WFS Layer part] #10748
base: master
Are you sure you want to change the base?
Conversation
…art] Description: - handle the functionality of interactive legend for WFS layers - create geostyler converter to cql - handle saving the legend filter of layer into layerFilter object in saving - handle showing incompatible message in case change into filtered style - add unit tests - add translations
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.
Please take a look to the inline comments.
Found an issue while testing locally:
- When experimentalInteractiveLegend config is false in the localConfig the interactive legend is active by default and the settings are showing the Legend title without content
legend-issue-01.mp4
<Col xs={12} className={"legend-label"}> | ||
<label key="legend-options-title" className="control-label"><Message msgId="layerProperties.legendOptions.title" /></label> | ||
</Col> | ||
{ experimentalInteractiveLegend && this.props.element?.serverType !== ServerTypes.NO_VENDOR && !this.props?.hideInteractiveLegendOption && |
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 don't think for WFS is important the serverType because the legend is rendered client side.
Please remove the this.props.element?.serverType !== ServerTypes.NO_VENDOR
|
||
describe('Test common advanced settings', () => { | ||
beforeEach((done) => { | ||
document.body.innerHTML = '<div id="container"></div>'; | ||
setConfigProp('miscSettings', { experimentalInteractiveLegend: true }); |
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.
If this is applied before each test we should also reset it after each test
const showResetWarning = !checkPreviousFiltersAreValid(rules, legendFilters) && !layerFilter.disabled; | ||
|
||
return (<> | ||
{showResetWarning && owner !== 'legendPreview' ? <Alert bsStyle="warning"> |
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.
We should use a boolean prop Instead of owner
because it seems to be used to enable the interactivity.
My proposal is to change owner
with interactive
and it will be a true just in the TOC
const isFilterDisabled = layer?.layerFilter?.disabled; | ||
const activeFilter = legendFilters?.some(f => f.id === rule?.filter?.toString()); | ||
return (<div key={`${rule.filter}-${idx}`} | ||
onClick={() => filterLayerHandler(rule.filter)} |
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.
if the legend is not interactive the onClick event should be removed
One of the following cases:
layer?.layerFilter?.disabled
true- owner === 'legendPreview' (
interactive
prop false) - !rule?.filter
} | ||
return ''; | ||
} | ||
let geostylerRules = geostylerFilter.body; |
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.
We should include a check to verify if the geostylerFilter has a body or it is an array filter passed directly, this is needed to align it to other converters behavior, see the cql one where in that case there is a isString check, eg:
const geostylerRules = isArray(geostylerFilter) ? geostylerFilter : geostylerFilter?.body;
Please use const when variable is not re-assigned
Description
This PR includes handling the functionality of interactive legend for WFS layers for mapViewer/dashboards/geostories. User can enable interactive legend from catalog or layer settings. A geostyler converter is created in this PR to handle converting from geostyler to cql.
If legend filter is applied, then user makes any change to the style --> a warning should appear when the user comes back to the TOC after having changed the style.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
#10736
What is the current behavior?
#10736
What is the new behavior?
If user enables interactive legend for WFS layer via catalog or layer settings, user can filter by legend.
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information
It is experimental feature