-
Notifications
You must be signed in to change notification settings - Fork 126
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
Fix: Update of generate.dart and its template files. Fixed Offset, Translate and expressions arrays on iOS. #481
base: main
Are you sure you want to change the base?
Conversation
Added checks in LayerPropertyConverter.swift to correct property values, in cases of arrays and expressions.
Thanks for all the namespace/path fixes, I completely missed those in the generator. |
@kuhnroyal On Android everything was fine for line-patharray, but it only accepts expressions and not arrays, when using an array a console error appears with "array not allowed". Do we want to accept only expressions on iOS as well? Currently, with my edits, arrays are also allowed on iOS (I think this must be the correct way also on Android, for a better UX). |
@kuhnroyal I did some checking on Android, and well, the bug isn't only on iOS but also on Android. But on Android it doesn't cause a crash, just an error log:
These days I will fix the LayerPropertConverter.java, also applying some optimization fixes, in case I will update you with commits and comments here. |
…n Android. Minor fix on iOS template and generate.dart.
Hi guys, any update on this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally seems ok to me, and solves a bug in theory.
@@ -65,7 +74,16 @@ static PropertyValue[] interpretSymbolLayerProperties(Object o) { | |||
properties.add(PropertyFactory.textHaloBlur(expression)); | |||
break; | |||
case "text-translate": | |||
properties.add(PropertyFactory.textTranslate(expression)); | |||
if (jsonElement.isJsonArray()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe put all this logic into convertJsonToFloatArray
to avoid repetition?..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually don't remember why I did this, it was a while ago. Maybe because in some cases the conversion was handled differently, but currently it seems logical to put everything in convertJsonToFloatArray
.
Hi guys, this PR aims to solve the #480 issue that I opened recently.
I started by refactoring all template files, used in generate.dart, updating them because they were obsolete (there were still some mapbox references inside).
Next, I fixed some problems with the paths in generate.dart, which were also obsolete.
Testing #480, I realized that the problem wasn't only with the “line-dasharray” property, but with all arrays that were part of a linear expression or that were arrays of doubles (generally numbers).
I introduced some checks in LayerPropertyConverter.swift and now every array or expression, for every property (even on Offset and Translate, which work with the CGVector class), works correctly on the iOS platform.