Upload QML (Issue 3433)

[raitisbe]

Description

Support upload of QML in style panel and create a walkaround for invalid WellKnownName problem when parsing qml with geostyler-qml-parser.

Related issues or pull requests

#3433
#3431

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Dependency updates
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe)
• I am unsure (we'll look into it together)

Do you introduce a breaking change?

• Yes
• No
• I am unsure (no worries, we'll find out)

Checklist

• I understand and agree that the changes in this PR will be licensed under the [MIT License]
• I have followed the guidelines for contributing
• The proposed change fits to the content of the code of conduct
• I have added or updated tests and documentation, and the test suite passes (run npm test locally)
• I'm lost; why do I have to check so many boxes? Please help!

[jmacura]

This is now in the console, when I start the test-app. I believe it was not there before...

UPDATE: Forgotten npm i

[jmacura]

Not much has changed regarding #3431

Only now when I add the layer "bioregion_pilots" from the data catalogue as WFS, all layers disappear from the map 😱

[raitisbe]

Yes, getcolor was added back in ol-ext just in the latest ol-ext version. Dicd you npm ci?

…

On Mon, 10 Oct 2022, 23:34 jmacura, ***@***.***> wrote: ***@***.**** requested changes on this pull request. [image: image] <https://user-images.githubusercontent.com/5598693/194948223-640a657e-5433-4e6a-bf2e-6aa2217fad3f.png> This is now in the console, when I start the test-app. I believe it was not there before... — Reply to this email directly, view it on GitHub <#3435 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADAR6QLZJJUM6EA4EU6UWTWCR4V3ANCNFSM6AAAAAARBG7DGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[jmacura]

Yes, getcolor was added back in ol-ext just in the latest ol-ext version. Dicd you npm ci?
…

Good point. My apologies. I have updated the comments above.

[raitisbe]

Not much has changed regarding #3431 Only now when I add the layer "bioregion_pilots" from the data catalogue as WFS, all layers disappear from the map scream

Right, I actually only fixed it when uploading the SLD to style pannel. Not for loading the layer from add-data

[raitisbe]

@jmacura could you retest please?

[jmacura]

Sure, will test it later today.

[jmacura]

The good news is that the error is gone 👏

This is how the styled layer looks like now.

Whereas it should look like this.

Not sure if it is an acceptable behaviour 😄

[jmacura]

The reason is that the Text Symbolizer is malformed.

The template for displaying the text is

whereas the correct formula should be

Not sure if we can fix this. I have to check what is actually stored in the layer's style..

[jmacura]

And... We can't. There is an issue opened in the geostyler/geostyler-qgis-parser library about it: geostyler/geostyler-qgis-parser#259
Expession based labels are not supported.

From the code in https://github.com/geostyler/geostyler-qgis-parser/blob/ae5203392377d2342ef04a50766eedcdfe5990ec/src/QGISStyleParser.ts#L143 it is quite obvious, that the "fieldName" attribute of the "text-style" element in QML gets misinterpreted as (.

<text-style textColor="51,160,44,255" multilineHeight="1" fieldName="concat(&quot;Country&quot;, ' (', &quot;count&quot;, ')')" fontWeight="87" fontItalic="0" fontStrikeout="0" allowHtml="0" previewBkgrdColor="255,255,255,255" isExpression="1" namedStyle="obyčejné" fontKerning="1" fontSize="10" fontFamily="Arial Black" fontSizeMapUnitScale="3x:0,0,0,0,0,0" blendMode="0" fontLetterSpacing="0" legendString="Aa" fontWordSpacing="0" capitalization="0" useSubstitutions="0" textOrientation="horizontal" fontUnderline="0" textOpacity="1" fontSizeUnit="Point">

FTR. Changing the label formatting expression from concat("Country", ' (', "count", ')') to Country || ' (' || count || ')' fixes this other issue and gets parsed correctly by geostyler-qgis-parser. (The TODO note in the code of geostyler/geostyler-qgis-parser https://github.com/geostyler/geostyler-qgis-parser/blob/ae5203392377d2342ef04a50766eedcdfe5990ec/src/QGISStyleParser.ts#L405 is actually obsolete. It DOES support the double pipe syntax. It DOES NOT support any more complex syntax. See issue geostyler/geostyler-qgis-parser#259)

Hence I think this PR is an acceptable fix of #3431 .

[jmacura]

Would you mind changing the last commit message to something better than "squash", @raitisbe ? 🙏 🙂

[jmacura]

One more thought...
If we will also allow to upload the QML style files along with a new layer in the Datasources panel, will it work out of the box now?

[jmacura]

We can also implement the upload of QML along with new vector layer later on.

[raitisbe]

long with new vector layer later on.

I did it in this PR, but It had so many changes I'm not sure I haven't broken something... Especially on the upload to Layman side. I think we will need to have some e2e test for this

[jmacura]

long with new vector layer later on.

I did it in this PR, but It had so many changes I'm not sure I haven't broken something... Especially on the upload to Layman side. I think we will need to have some e2e test for this

Aye. That is what I was afraid of. Actually I started to implement it after our telco today, but then I have realised I am lost, as I do not know the consequences on the Layman side so I have thrown my changes away.
Will test your changes, thanks!

[jmacura]

Works like a charm 🚀