Textsymbolizer from OlStyle #14
Conversation
…zers from OlStyle with text property
There was a problem hiding this comment.
Thank you very much for this @annarieger! Looks good to me. One little point regarding the font parser should be changed before merging, IMHO.
For me it would be enough if we agree on the simple cases taking the font-weight into respect, like bold 12px serif. More sophisticated cases also appreciated of course 😉
src/OlStyleParser.ts
Outdated
| const offsetX = olTextStyle.getOffsetX(); | ||
| const offsetY = olTextStyle.getOffsetY(); | ||
| const font = olTextStyle.getFont(); | ||
| const fontSize = parseInt(font.split('px')[0], 10); |
There was a problem hiding this comment.
Since font in an ol.style.Text is a CSS 'font' value this does not seem to work for all cases. E. g. when a font-weight is set: bold 5px arial, sans-serif
Even more complicated cases are thinkable: italic bold 12px/30px Georgia, serif. See https://www.w3schools.com/cssref/pr_font_font.asp for more infomations.
There was a problem hiding this comment.
In 62dd6df I added some logic to parse even more complex CSS font values.
There was a problem hiding this comment.
However, this currently only works with font-size values given in pixel.
Since font-size can be a string value as well (see https://www.w3schools.com/cssref/pr_font_font-size.asp, e.g. large) the model of TextSymbolizer (size?: number;) should be changed. This is is beyond the scope of this PR, IMHO. What do you think @chrismayer @KaiVolland ?
There was a problem hiding this comment.
I don't think we need to support this atm. This would make many things very complicated.
But we should improve the feedback of the parser. Maybe return an feedback object or throw errors. I created a ticket regarding this: geostyler/geostyler#286
src/OlStyleParser.ts
Outdated
| const font = olTextStyle.getFont(); | ||
|
|
||
| // font-size is always the first part of font-size/line-height | ||
| const fontStyleWightSize: string = font.split('px')[0].trim(); |
|
Thx for review. I'll merge as soon as Travis is happy. |
This PR containing commits by @annarieger implementing getTextSymbolizerFromOlStyle function to parse text symbolizer from ol style with set text property.
Credits go to @annarieger
Please review
Follow up to #13