feat{geometry-form-field}: allow to set symbol

[ameliebernier]

Please check if the PR fulfills these requirements

• [ x ] The commit message follows our guidelines:https://github.com/infra-geo-ouverte/igo2/blob/master/.github/CONTRIBUTING.md#git-commit-guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

What is the current behavior? (You can also link to an open issue here)

It's not possible to define custom symbol for geometry input draw tool.
Draw tool cannot be deactivated.
When draw guide is undefined, radius is 0 and no point appears when drawing / also point added to map is too small to be noticeable.

What is the new behavior?

geometry-form-field and geometry-form-field-input now allows to:

• Set color of symbol
• Set icon as point symbol
• Deactivate draw tool by setting null geometry type
• Apply default circle radius of 8 when no draw guide is defined

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[ x ] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications:

Other information:

[cbourget]

Wouldn't it be better to add a "drawStyle" input instead of "symbolColor" and "pointIcon"? I feel like we'll want to do define more elaborate styles in the future and this will come as a limitation whereas a "drawStyle" input will allow us to uses OL to it's full extent.

[ameliebernier]

It is because of the drawing guide that necessitates a circle image with radius. If the symbol for point is not a circle image then the draw guide would not work.

Or we could force the use of circle if draw guide is defined, then replace with the real symbol when the feature is added to the map.

[cbourget]

I get why you check if the geometryType is defined but I think it would be more "futureproof" to check if the drawControl itself is defined.

[ameliebernier]

Draw control was still defined even with invalid geometry type

[cbourget]

I think this would be solved with the modification suggested above.

[cbourget]

I think the createDrawControl in ngOnInit could be harmlessly removed and the check for a defined geometryType moved to the geometryType setter. This would remove the need for a check here.

[ameliebernier]

So no tool would be activated by default? Because if there is a geometryType defined on init the tool should be activated I think

[cbourget]

ngOnInit is invoked after the inputs have been resolved so there should still be at least one control active.

[ameliebernier]

I tried but it knida breaks the logic, there is a condition in the geometry type setter which returns when onInit is not completed

[ameliebernier]

Should I keep as it is or change that also

[cbourget]

I think it could be safely removed from the geometryType setter. I have been working on a version of this component that completely removes the need for the "ready" flag. I might push it later but, for now, let's remove it from the setter only, if possible.

[ameliebernier]

It does not simplify things a lot I fear... Draw control cannot be created before overlay layer is added to the map

[ameliebernier]

If I do some kind of default vector layer is hooked to the draw control and the first geometry is added twice

[cbourget]

I understand. The drawGuide is something quite niche. We could check wheter the symbol for points is an instance of OlStyle.Circle and ignore it if it's not.

Is the intent of the PR to make the "draw style" configurable or, the style of the feature once it's drawn and added to the map?

[ameliebernier]

As it is now the draw symbol and symbol on the map are the same. In our use case they would have to match. Would it be better to have two symbols?

• One symbol defined by user (used to represent the geometries on the map)
• One symbol for drawing (user defined symbol image is overriden with circle if there is a draw guide defined)

However I'm not sure if this would work for any symbol possible. Could the point symbol be another thing than Image?

[cbourget]

The "draw style" and "layer style" are different. The draw style is used when drawing, then, the layer's style takes over. They look the same because the default draw style uses the same color as the default OL style.

The draw/modify control used in the draw field/input may receive a layer to add/modify features to/from. Likewise, the geometry field/input could receive a layer input. The drawStyle and the layer's style could be identical to achieve what you want. Does that make sense?

We could replace the draw symbol with a circle when a drawGuide is defined but I don't think this is absolutely required. Also, whether we replace it or not, we should do nothing if the style provided is a style function instead of a style instance.

[cbourget]

I just thought of something. Maybe the "draw guide" could have his own geometry and his own style. That way, we could style it however we want without taking care of the draw style.

This is probably out of this PR's scope so, for now, we could simply ignore the drawGuide if the supplied draw style is a style functiun or if it's a style instance without a circle. What do you think?

[ameliebernier]

Yes they are different, but in the current state both uses the same style. I removed the default vector layer style to apply the draw style to the geometry once added to the map. I think in most case the user would want both to be identical?

[ameliebernier]

I think it would be easier to do just that, drawStyle and geometryStyle could be defined by user, or else default style applies

[ameliebernier]

We would not need two layers

[cbourget]

Alright, so let's go with two styles: drawStyle and overlayStyle. If the "drawStyle" is defined and the "overlayStyle" is not, we could fallback to the "drawStyle". Also, the "overlayStyle" could be applied that the layer level instead of the feature level.

[ameliebernier]

I did another commit with drawSymbol and overlaySymbol inputs. Draw guide applies if possible. I could not remove completely the isReady check in geomteryType setter, it broke the logic in some places but mainly you have to wait for the vector layer to be created to create any draw control, or else it hooks to default map vector layer. I would wait for @cbourget new version of the control.

I also had to set overlayLayer style at the feature level, if set at the layer level symbology applies twice for each feature. I can confirm features are not added twice, but could not find a reason for this.

@mbarbeau

[cbourget]

Alright, looks good to me. You've integrated the "drawStyle" and "overlayStyle" inputs. The "isReady" stuff wasn't crucial so let's roll with this.

[cbourget]

Good job