Comparison Filter GUI #64

chrismayer · 2018-05-16T13:40:54Z

This introduces a GUI visualizing a ComparisonFilter. The GUI consists of a bunch of react components.

For demonstration purpose a little demo app has also been developed, where the following can be done:

Select a GeoJSON file and import it into the app
Select an attribute, an operator and a value to define the ComparisonFilter
Dependent on the attribute type (String, Number, Boolean) a different UI is shown
The internal ComparisonFilter object can be serialized and is printed into a text field

UI for boolean attributes:

This adds several components, which can be used to aggregate to a Comparison Filter UI. For demonstration purposes a basic Demo UI is also implemented. (Addresses #58)

Adds an UploadButton component which let the user select a geodata file as base for the filter UI.

… into 58-filter-gui

This enables the ComparisonFilterUi to hold a GeoStyler ComparisonFilter object derived from its GUI elements.

This adds a text area to output the serialized GeoStyler filter in the FilterDemoUi.

Disable button to add more ComparisonFilterUis since we cannot handle multiple filters at the moment.

KaiVolland · 2018-05-17T08:26:03Z

src/Component/Filter/AttributeCombo/AttributeCombo.tsx

+ */
+class AttributeCombo extends React.Component<any, any> {
+
+  // PROPS


React usually defines its props with PropTypes.
As we make use of TypeScript another approach with interfaces should be the way to go.

The props should not be declared as member variables. The declared member variables will not be associated with the props. So you will have this.internalDataDef and this.props.internalDataDef and so on.

Good point, I have this on my list. This is the next thing I will tackle.

KaiVolland · 2018-05-17T08:39:23Z

src/Component/Filter/AttributeCombo/AttributeCombo.tsx

+
+      </div>
+    );
+  }


You might want to profit from object destructuring / spread operator for the props. You can then use "passThorughProps".
Compare

KaiVolland · 2018-05-17T08:39:49Z

src/Component/Filter/ComparisonFilter/ComparisonFilterUi.css

@@ -0,0 +1,4 @@
+


Please use less instead of css

I created a separate issue for this (#65), so we can tackle this later (since this seems a bit more complicated that one might think at first sight).

KaiVolland · 2018-05-17T08:40:44Z

src/Component/Filter/ComparisonFilter/ComparisonFilterUi.tsx

+ *   - A combo to select the operator
+ *   - An input field for the value
+ */
+class ComparisonFilterUi extends React.Component<any, any> {


Please remove "Ui" from the classname

KaiVolland · 2018-05-17T08:41:09Z

src/Component/Filter/ComparisonFilter/ComparisonFilterUi.tsx

+ */
+class ComparisonFilterUi extends React.Component<any, any> {
+
+  // PROPS


See comment above regarding props.

KaiVolland · 2018-05-17T08:43:56Z

src/Component/Filter/ComparisonFilter/ComparisonFilterUi.tsx

+   *
+   * Changes the input field for the filter value and stores the appropriate attribute name as member.
+   */
+  onAttributeChange = (newAttrName: string) => {


Please transform methods to following syntax and use .bind(this) at the call if needed:

onAttributeChange(newAttrName: string) {

Otherwise styleguidist doesn't recognize this methods as methods but as properties.

I will check that

At the moment binds are forbidden in JSX attributes due to their rendering performance impact (tslint).
So what do you prefer? Making styleguidist happy or relying on the tslint recommendation not to use bind?

I wasn't aware that TypeScript recommends this way of declaring class methods. So
we should stick to the TypeScript recommendation for know.

KaiVolland · 2018-05-17T08:48:15Z

src/Component/Filter/AttributeCombo/AttributeCombo.tsx

@@ -0,0 +1,77 @@
+import * as React from 'react';


I'm not sure if it makes a difference but i'd prefer import React from 'react';.

Does not work for me. The module doesn't seem to have a standard export. And using import { React } from 'react'; also does not work because there is no exported member React in the module.
I also just discovered import * as React from 'react'; in all documentations / tutorials I read so far.

It should work if you leave the curly-brackets. Never mind. It's just Schmuck at the Nachthemd...

KaiVolland · 2018-05-17T08:53:21Z

src/Component/Filter/AttributeCombo/AttributeCombo.tsx

+  /** Handler function bound to the 'onChange' of the underlying Select */
+  onAttributeChange: Function;
+
+  constructor(props: any) {


If you don't perform any actrions with props or the state you can omit the constructor.

KaiVolland · 2018-05-17T08:55:25Z

Looks very promising. 😍 I added some notes to structural things.
Would be cool if you could add a screenshot once your happy with the visualization.

chrismayer · 2018-05-17T09:44:43Z

Thanks for your review @KaiVolland! Good points, I agree with most of them. I will go on and address your remarks / hints.

This reworks the filter UI components so the react properties are modelled corretly for TypeScript. Concept taken from: https://charleslbryant.gitbooks.io/hello-react-and-typescript/content/TypeScriptAndReact.html

Detects the min and max value from the attribute schema for numeric attributes.

Adds a BoolFilterField component offering a checkbox to set the filter value for attributes of type boolean .

chrismayer · 2018-05-17T13:46:53Z

I addressed or at least commented your remarks, @KaiVolland. I think this is now ready for another (and hopefully final) review.

KaiVolland

Very nice. Feel free to merge.

Could you explain in one or two sentences (or maybe a source) why you decide to split the props interface in "default props" and "non default props"?

KaiVolland · 2018-05-18T06:46:27Z

src/Component/Filter/AttributeCombo/AttributeCombo.tsx

@@ -0,0 +1,77 @@
+import * as React from 'react';


It should work if you leave the curly-brackets. Never mind. It's just Schmuck at the Nachthemd...

KaiVolland · 2018-05-18T06:58:22Z

src/Component/Filter/ComparisonFilter/ComparisonFilter.tsx

+  operator: ComparisonOperator;
+
+  /** The currently entered filter value */
+  value: string | number | boolean | null;


Maybe the 4 props above should go to the state, too. But we can adjust this later.

I don't think so. Since setting the state is an asynchronous action (by triggering the rendering not immediately) this can lead to divergence in the values for attribute, operator and value, when serializing them. Holding them as "normal" members ensures the correct values at every time.
For my understanding the state should be used for information, which is directly connected to a component.

chrismayer · 2018-05-18T07:30:35Z

Thanks again for your review @KaiVolland!

Could you explain in one or two sentences (or maybe a source) why you decide to split the props interface in "default props" and "non default props"?

The "default props" are these props, which are optional and have a default value. The other have to be passed in by the father component. By modeling the "default props" in an own interface we can define a static defaultProps element in our component class to set the default values.

  public static defaultProps: DefaultAttributeComboProps = {
    label: 'Attribute',
    placeholder: 'Select Attribute',
  };

I found several sources describing this, e.g.:

https://stackoverflow.com/questions/37282159/default-property-value-in-react-component-using-typescript/37282264

chrismayer added 8 commits May 15, 2018 14:56

Move DataProviderUi to own folder

6ba9b01

Add antd and geostyler-style as dependency

0aa68a1

Implement a Comparison Filter GUI

6b90eee

This adds several components, which can be used to aggregate to a Comparison Filter UI. For demonstration purposes a basic Demo UI is also implemented. (Addresses #58)

Implement a Comparison Filter GUI

d240363

This adds several components, which can be used to aggregate to a Comparison Filter UI. For demonstration purposes a basic Demo UI is also implemented. (Addresses #58)

Add button to import geodata in FilterDemo

2cfc9ba

Adds an UploadButton component which let the user select a geodata file as base for the filter UI.

Merge branch '58-filter-gui' of https://github.com/chrismayer/geostyler…

6308839

… into 58-filter-gui

Create ComparisonFilter object from GUI

af6a723

This enables the ComparisonFilterUi to hold a GeoStyler ComparisonFilter object derived from its GUI elements.

Add text output for generated filter in Demo UI

89085d3

This adds a text area to output the serialized GeoStyler filter in the FilterDemoUi.

chrismayer force-pushed the 58-filter-gui branch from 8603546 to 89085d3 Compare May 17, 2018 07:33

Disable 'Add ComparisonFilterUi' button

97d9040

Disable button to add more ComparisonFilterUis since we cannot handle multiple filters at the moment.

KaiVolland reviewed May 17, 2018

View reviewed changes

chrismayer added 5 commits May 17, 2018 12:54

Use the react properties the TypeScript way

014dad3

This reworks the filter UI components so the react properties are modelled corretly for TypeScript. Concept taken from: https://charleslbryant.gitbooks.io/hello-react-and-typescript/content/TypeScriptAndReact.html

Remove 'UI'-suffix from filter component class names

e5c6d55

Minor style adaptations in ComparisonFilter GUI

481c562

Detect min / max values in NumberFilterField

8d4eb9b

Detects the min and max value from the attribute schema for numeric attributes.

Add BoolFilterField component

69d7f8a

Adds a BoolFilterField component offering a checkbox to set the filter value for attributes of type boolean .

chrismayer changed the title ~~[WIP] Comparison Filter GUI~~ Comparison Filter GUI May 17, 2018

chrismayer mentioned this pull request May 17, 2018

UI for combined ComparisonFilter objects #69

Closed

chrismayer added 2 commits May 18, 2018 07:41

Update geostyler-style to v0.2.0

3260c4a

Improve API-docs for ComparisonFilter GUI

4c27d6e

KaiVolland approved these changes May 18, 2018

View reviewed changes

chrismayer merged commit e531962 into geostyler:master May 18, 2018

chrismayer mentioned this pull request May 18, 2018

Comparison Filter GUI #58

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comparison Filter GUI #64

Comparison Filter GUI #64

chrismayer commented May 16, 2018 •

edited

KaiVolland May 17, 2018

chrismayer May 17, 2018

KaiVolland May 17, 2018

KaiVolland May 17, 2018

chrismayer May 17, 2018

chrismayer May 17, 2018

KaiVolland May 17, 2018

chrismayer May 17, 2018

KaiVolland May 17, 2018

KaiVolland May 17, 2018 •

edited

chrismayer May 17, 2018

chrismayer May 17, 2018

KaiVolland May 17, 2018

KaiVolland May 17, 2018

chrismayer May 17, 2018

KaiVolland May 18, 2018

KaiVolland May 17, 2018

chrismayer May 17, 2018

KaiVolland commented May 17, 2018

chrismayer commented May 17, 2018

chrismayer commented May 17, 2018

KaiVolland left a comment

KaiVolland May 18, 2018

KaiVolland May 18, 2018

chrismayer May 18, 2018

chrismayer commented May 18, 2018

Comparison Filter GUI #64

Comparison Filter GUI #64

Conversation

chrismayer commented May 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KaiVolland May 17, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KaiVolland commented May 17, 2018

chrismayer commented May 17, 2018

chrismayer commented May 17, 2018

KaiVolland left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrismayer commented May 18, 2018

chrismayer commented May 16, 2018 •

edited

KaiVolland May 17, 2018 •

edited