-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Maps] move source details to Panel header #29298
Changes from 7 commits
6df0774
76fb38c
abdce9f
3829f6b
4f855e0
f9250d2
e17e4dd
b480b91
16bd9b8
cc6b793
6790746
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
* you may not use this file except in compliance with the Elastic License. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ugh, I absolute hate that function syntax for a react component. so cluttered. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But it is more performant since there are not state checks or life cycle methods There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. interesting. but if we want to avoid React internally to do this whole life-cycle loop (since internally, it does go through it even when using function-components), we would also need to re-implement how we include the component. I just got this from: |
||
*/ | ||
|
||
import React, { Component, Fragment } from 'react'; | ||
import React from 'react'; | ||
|
||
import { | ||
EuiFlexGroup, | ||
|
@@ -15,46 +15,35 @@ import { | |
EuiFieldText, | ||
EuiRange, | ||
EuiSpacer, | ||
EuiLink, | ||
} from '@elastic/eui'; | ||
import { ValidatedRange } from '../../../shared/components/validated_range'; | ||
|
||
export class SettingsPanel extends Component { | ||
export function SettingsPanel(props) { | ||
|
||
state = { | ||
showSourceDetails: false, | ||
} | ||
|
||
toggleSourceDetails = () => { | ||
this.setState((prevState) => ({ | ||
showSourceDetails: !prevState.showSourceDetails, | ||
})); | ||
} | ||
|
||
onLabelChange = (event) => { | ||
const onLabelChange = (event) => { | ||
const label = event.target.value; | ||
this.props.updateLabel(this.props.layerId, label); | ||
} | ||
props.updateLabel(props.layerId, label); | ||
}; | ||
|
||
onMinZoomChange = (event) => { | ||
const onMinZoomChange = (event) => { | ||
const zoom = parseInt(event.target.value, 10); | ||
this.props.updateMinZoom(this.props.layerId, zoom); | ||
} | ||
props.updateMinZoom(props.layerId, zoom); | ||
}; | ||
|
||
onMaxZoomChange = (event) => { | ||
const onMaxZoomChange = (event) => { | ||
const zoom = parseInt(event.target.value, 10); | ||
this.props.updateMaxZoom(this.props.layerId, zoom); | ||
} | ||
props.updateMaxZoom(props.layerId, zoom); | ||
}; | ||
|
||
onAlphaChange = (alpha) => { | ||
this.props.updateAlpha(this.props.layerId, alpha); | ||
} | ||
const onAlphaChange = (alpha) => { | ||
props.updateAlpha(props.layerId, alpha); | ||
}; | ||
|
||
onSourceChange = ({ propName, value }) => { | ||
this.props.updateSourceProp(this.props.layerId, propName, value); | ||
} | ||
const onSourceChange = ({ propName, value }) => { | ||
props.updateSourceProp(props.layerId, propName, value); | ||
}; | ||
|
||
renderZoomSliders() { | ||
const renderZoomSliders = () => { | ||
return ( | ||
<EuiFormRow | ||
helpText="Display layer when map is within zoom level range." | ||
|
@@ -67,8 +56,8 @@ export class SettingsPanel extends Component { | |
<EuiRange | ||
min={0} | ||
max={24} | ||
value={this.props.minZoom.toString()} | ||
onChange={this.onMinZoomChange} | ||
value={props.minZoom.toString()} | ||
onChange={onMinZoomChange} | ||
showInput | ||
showRange | ||
/> | ||
|
@@ -81,8 +70,8 @@ export class SettingsPanel extends Component { | |
<EuiRange | ||
min={0} | ||
max={24} | ||
value={this.props.maxZoom.toString()} | ||
onChange={this.onMaxZoomChange} | ||
value={props.maxZoom.toString()} | ||
onChange={onMaxZoomChange} | ||
showInput | ||
showRange | ||
/> | ||
|
@@ -91,22 +80,22 @@ export class SettingsPanel extends Component { | |
</EuiFlexGroup> | ||
</EuiFormRow> | ||
); | ||
} | ||
}; | ||
|
||
renderLabel() { | ||
const renderLabel = () => { | ||
return ( | ||
<EuiFormRow | ||
label="Layer display name" | ||
> | ||
<EuiFieldText | ||
value={this.props.label} | ||
onChange={this.onLabelChange} | ||
value={props.label} | ||
onChange={onLabelChange} | ||
/> | ||
</EuiFormRow> | ||
); | ||
} | ||
}; | ||
|
||
renderAlphaSlider() { | ||
const renderAlphaSlider = () => { | ||
return ( | ||
<EuiFormRow | ||
label="Layer transparency" | ||
|
@@ -116,59 +105,35 @@ export class SettingsPanel extends Component { | |
min={.00} | ||
max={1.00} | ||
step={.05} | ||
value={this.props.alpha} | ||
onChange={this.onAlphaChange} | ||
value={props.alpha} | ||
onChange={onAlphaChange} | ||
showLabels | ||
showInput | ||
showRange | ||
/> | ||
</div> | ||
</EuiFormRow> | ||
); | ||
} | ||
}; | ||
|
||
renderSourceDetails() { | ||
if (!this.state.showSourceDetails) { | ||
return null; | ||
} | ||
return ( | ||
<EuiPanel> | ||
<EuiFlexGroup> | ||
<EuiFlexItem> | ||
<EuiTitle size="xs"><h5>Settings</h5></EuiTitle> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
|
||
return ( | ||
<Fragment> | ||
{this.props.renderSourceDetails()} | ||
<EuiSpacer margin="m"/> | ||
</Fragment> | ||
); | ||
} | ||
<EuiSpacer margin="m"/> | ||
|
||
render() { | ||
return ( | ||
<EuiPanel> | ||
<EuiFlexGroup> | ||
<EuiFlexItem> | ||
<EuiTitle size="xs"><h5>Settings</h5></EuiTitle> | ||
</EuiFlexItem> | ||
<EuiFlexItem grow={false}> | ||
<EuiLink | ||
onClick={this.toggleSourceDetails} | ||
> | ||
source details | ||
</EuiLink> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
|
||
<EuiSpacer margin="m"/> | ||
{renderLabel()} | ||
|
||
{this.renderSourceDetails()} | ||
{renderZoomSliders()} | ||
|
||
{this.renderLabel()} | ||
{renderAlphaSlider()} | ||
|
||
{this.renderZoomSliders()} | ||
{props.renderSourceSettingsEditor({ onChange: onSourceChange })} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. after merging this will become |
||
|
||
{this.renderAlphaSlider()} | ||
|
||
{this.props.renderSourceSettingsEditor({ onChange: this.onSourceChange })} | ||
|
||
</EuiPanel> | ||
); | ||
} | ||
</EuiPanel> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,17 +21,38 @@ import { | |
EuiFlyoutBody, | ||
EuiFlyoutFooter, | ||
EuiSpacer, | ||
EuiAccordion, | ||
EuiText, | ||
EuiLink, | ||
} from '@elastic/eui'; | ||
|
||
export class LayerPanel extends React.Component { | ||
|
||
state = { | ||
displayName: null | ||
static getDerivedStateFromProps(nextProps, prevState) { | ||
const nextId = nextProps.selectedLayer.getId(); | ||
if (nextId !== prevState.prevId) { | ||
return { | ||
displayName: '', | ||
immutableSourceProps: [], | ||
hasLoadedSourcePropsForLayer: false, | ||
prevId: nextId, | ||
}; | ||
} | ||
|
||
return null; | ||
} | ||
|
||
state = {} | ||
|
||
componentDidMount() { | ||
this._isMounted = true; | ||
this.loadDisplayName(); | ||
this.loadImmutableSourceProperties(); | ||
} | ||
|
||
componentDidUpdate() { | ||
this.loadDisplayName(); | ||
this.loadImmutableSourceProperties(); | ||
} | ||
|
||
componentWillUnmount() { | ||
|
@@ -40,8 +61,24 @@ export class LayerPanel extends React.Component { | |
|
||
loadDisplayName = async () => { | ||
const displayName = await this.props.selectedLayer.getDisplayName(); | ||
if (!this._isMounted || displayName === this.state.displayName) { | ||
return; | ||
} | ||
|
||
this.setState({ displayName }); | ||
} | ||
|
||
loadImmutableSourceProperties = async () => { | ||
if (this.state.hasLoadedSourcePropsForLayer) { | ||
return; | ||
} | ||
|
||
const immutableSourceProps = await this.props.selectedLayer.getImmutableSourceProperties(); | ||
if (this._isMounted) { | ||
this.setState({ displayName }); | ||
this.setState({ | ||
immutableSourceProps, | ||
hasLoadedSourcePropsForLayer: true, | ||
}); | ||
} | ||
} | ||
|
||
|
@@ -60,6 +97,23 @@ export class LayerPanel extends React.Component { | |
); | ||
} | ||
|
||
_renderSourceProperties() { | ||
return this.state.immutableSourceProps.map(({ label, value, link }) => { | ||
function renderValue() { | ||
if (link) { | ||
return (<EuiLink href={link} target="_blank">{value}</EuiLink>); | ||
} | ||
return (<span>{value}</span>); | ||
} | ||
return ( | ||
<p key={label}> | ||
<strong className="gisLayerDetails__label">{label}</strong> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can remove this class and remove the span around the value since I was trying to do some line-up trickery but it looks like it won't work in most cases including the screenshot you've got. So just let them line-up normally. |
||
{renderValue()} | ||
</p> | ||
); | ||
}); | ||
} | ||
|
||
render() { | ||
const { selectedLayer } = this.props; | ||
|
||
|
@@ -81,6 +135,16 @@ export class LayerPanel extends React.Component { | |
</EuiTitle> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
<EuiAccordion | ||
id="accordion1" | ||
buttonContent="Source details" | ||
> | ||
<EuiText color="subdued" size="s"> | ||
<div className="gisLayerDetails"> | ||
{this._renderSourceProperties()} | ||
</div> | ||
</EuiText> | ||
</EuiAccordion> | ||
</EuiFlyoutHeader> | ||
|
||
<EuiFlyoutBody className="gisLayerPanel__body"> | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,9 +131,9 @@ export class AbstractLayer { | |
return this._style; | ||
} | ||
|
||
renderSourceDetails = () => { | ||
return this._source.renderDetails(); | ||
}; | ||
async getImmutableSourceProperties() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice rename |
||
return this._source.getImmutableProperties(); | ||
} | ||
|
||
renderSourceSettingsEditor = ({ onChange }) => { | ||
return this._source.renderSourceSettingsEditor({ onChange }); | ||
|
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.
should be careful for a couple small merge conflicts with https://github.com/elastic/kibana/pull/29294/files