-
Notifications
You must be signed in to change notification settings - Fork 191
[sdk] Expand DSL capabilities of ExpressionArgumentBuilder #2355
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
[sdk] Expand DSL capabilities of ExpressionArgumentBuilder #2355
Conversation
|
Looks great! Thank you, @johnhammerlund P.S. (just for me) The internal ticket for this request was MAPSIOS-413 |
|
Also congrats on your first PR 🎉 |
|
Hello @OdNairy , any updates on this? Thanks! |
|
Hey @johnhammerlund |
Original PR: - #2355 Resolves MAPSIOS-413 `ExpressionArgumentBuilder` currently only supports the basic `buildBlock(...)` method, but not loop expressions or conditional expressions. There are cases where these can simplify logic, reduce constants, etc. For example, a `SymbolLayer` may have a large matrix of icons: ```swift let iconImageExp = Exp(.match) { "make" "ford" Exp(.match) { "color" "red" "red-ford-icon" "green" "green-ford-icon" } "chevy" Exp(.match) { ... } } ``` We could instead leverage Swift a bit more: ```swift let iconImageExp = Exp(.match) { "make" for make in CarMake.allCases { make.featureValue Exp(.match) { "color" for color in CarColor.allCases { color.featureValue make.iconID(color: color) } } } } ``` The second example is not currently possible. This PR adds support for loops/arrays as well as conditionals. Some feature examples have been updated to leverage this, but happy to revert that if desired. ## Pull request checklist: - [x] Describe the changes in this PR, especially public API changes. - [ ] Include before/after visuals or gifs if this PR includes visual changes. <!-- | Before | After | | ----- | ----- | | <img src="" width = 250/> | <img src="" width = 250/> | or | <video src="" width = 250/> | <video src="" width = 250/> | --> - [x] Write tests for all new functionality. Put tests in correct [Test Plan](https://github.com/mapbox/mapbox-maps-ios/tree/main/Tests/TestPlans) (Unit, Integration, All) - [ ] If tests were not written, please explain why. - [ ] Add documentation comments for any added or updated public APIs. - [ ] Add any new public, top-level symbols to the DocC custom catatlog (Sources/MapboxMaps/Documentation.docc/API Catalogs) - [ ] Add a changelog entry to to bottom of the relevant section (typically the `## main` heading near the top). - [ ] Update the guides (internal access only), README.md, and DEVELOPING.md if their contents are impacted by these changes. - [ ] If this PR is a `v10.[version]` release branch fix / enhancement, merge it to `main` first and then port to `v10.[version]` release branch. PRs must be submitted under the terms of our Contributor License Agreement [CLA](https://github.com/mapbox/mapbox-maps-ios/blob/main/CONTRIBUTING.md#contributor-license-agreement). cc @mapbox/maps-ios cc @mapbox/sdk-ci --------- Co-authored-by: John Hammerlund <[email protected]> GitOrigin-RevId: 1a2bb8e04b44dadf8e1bd901b8b9e6429865daad
|
@johnhammerlund Heads up that PR was merged and available in the main branch: Thank you very much for such a nice addition! |
|
Excellent, appreciate it! Thanks @OdNairy 👍 |
ExpressionArgumentBuildercurrently only supports the basicbuildBlock(...)method, but not loop expressions or conditional expressions. There are cases where these can simplify logic, reduce constants, etc. For example, aSymbolLayermay have a large matrix of icons:We could instead leverage Swift a bit more:
The second example is not currently possible. This PR adds support for loops/arrays as well as conditionals. Some feature examples have been updated to leverage this, but happy to revert that if desired.
Pull request checklist:
## mainheading near the top).v10.[version]release branch fix / enhancement, merge it tomainfirst and then port tov10.[version]release branch.PRs must be submitted under the terms of our Contributor License Agreement CLA.