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

Geomap: Improve geojson style editor #41926

Merged
merged 9 commits into from
Nov 19, 2021
Merged

Conversation

nmarrs
Copy link
Contributor

@nmarrs nmarrs commented Nov 18, 2021

What this PR does / why we need it:

Improve geojson style editor in following ways:

  • When the features have points, we now show the symbol selection
  • Default style section is now above the rules selection
  • When adding a new rule a new color is randomly determined
  • Improve geojson rule editor for numeric properties
  • Add != comparator option
Geojson.UX.improvements.mov

Which issue(s) this PR fixes:

Fixes #41913

@nmarrs nmarrs added this to the 8.3.0-beta2 milestone Nov 18, 2021
@nmarrs nmarrs self-assigned this Nov 18, 2021
@nmarrs nmarrs marked this pull request as ready for review November 18, 2021 23:20
@nmarrs nmarrs requested a review from a team November 18, 2021 23:20
@ryantxu ryantxu added the backport v8.3.x Mark PR for automatic backport to v8.3.x label Nov 18, 2021
@@ -40,7 +40,21 @@ export const StyleRuleEditor: FC<StandardEditorProps<FeatureStyleConfig, any, an
const uniqueSelectables = useMemo(() => {
const key = value?.check?.property;
if (key && feats && value.check?.operation === ComparisonOperation.EQ) {
return getUniqueFeatureValues(feats, key).map((v) => ({ value: v, label: v }));
return getUniqueFeatureValues(feats, key).map((v) => {
Copy link
Member

Choose a reason for hiding this comment

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

hymm -- rather than adding this funky business, maybe we should check when calling onChangeValue?

The weird part is that the number vs string distinction should really come from the feature value, not the comparison value 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally had these checks in the onChangeValue but that resulted in the value select to have (not found) appended to each value due to number and string inequality. It seems that we want to store number values as numbers for best UX (hence during initialization step vs. later on)

@@ -30,6 +33,7 @@ import { defaultStyleConfig, StyleConfig, TextAlignment, TextBaseline } from '..
import { styleUsesText } from '../../style/utils';

export interface StyleEditorOptions {
features?: Observable<FeatureLike[]>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
features?: Observable<FeatureLike[]>;
layerInfo: Observable<LayerContentInfo>;

@@ -79,10 +83,43 @@ export const StyleEditor: FC<StandardEditorProps<StyleConfig, StyleEditorOptions
onChange({ ...value, textConfig: { ...value.textConfig, textBaseline: textBaseline as TextBaseline } });
};

let featuresHavePoints = false;
Copy link
Member

Choose a reason for hiding this comment

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

layerInfo has a geometryType property :)

@nmarrs nmarrs requested a review from ryantxu November 19, 2021 01:07
let featuresHavePoints = false;
if (item.settings?.layerInfo) {
const propertyOptions = useObservable(item.settings?.layerInfo);
featuresHavePoints = propertyOptions?.geometryType === 'point';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
featuresHavePoints = propertyOptions?.geometryType === 'point';
featuresHavePoints = propertyOptions?.geometryType === GeometryTypeId.Point;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, didn't see this suggestion

Copy link
Member

@ryantxu ryantxu left a comment

Choose a reason for hiding this comment

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

looks great!

@nmarrs nmarrs merged commit 541d154 into main Nov 19, 2021
@nmarrs nmarrs deleted the geomap-improve-geojson-style-editor branch November 19, 2021 01:39
nmarrs added a commit that referenced this pull request Nov 19, 2021
grafanabot pushed a commit that referenced this pull request Nov 19, 2021
leventebalogh added a commit that referenced this pull request Nov 19, 2021
…nents

* main: (22 commits)
  Refactor drone yaml (#41937)
  Update latest.json (#41873)
  Fix loop when cannot fetch roles (#41901)
  AzureMonitor: Fix metric namespace clear (#41878)
  Access Control: use role groups in role picker (#41912)
  AccessControl: RolePicker fetch roles in org (#41927)
  add unsupported renderer message to catalog (#41898)
  Install netcat to build-container and update the version (#41825)
  Transformers: extract fields from JSON and text (alpha) (#41791)
  Candlestick: fix volume histogram height by using mapped field name (#41931)
  Geomap: Improve geojson style editor (#41926)
  Docs: Add configuration option for the image renderer (#41798)
  Chore: Move babel config to a root babel.config.json (#41615)
  Make initialize depend on clone, only on enterprise pipelines (#41909)
  Run integration tests after initialize (#41906)
  Datasource: Fix stable sort order of query responses (#41868)
  Disable lint-drone from release pipelines (#41899)
  Add basic resource trimming command (#41780)
  Update grabpl version to 2.6.1 (#41892)
  Azure Monitor: Clean up fields when editing Metrics (#41762)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend area/panel/geomap backport v8.3.x Mark PR for automatic backport to v8.3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Geomap: Improve geojson style editor
3 participants