Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add
vertical-align
in format expression #900base: main
Are you sure you want to change the base?
Add
vertical-align
in format expression #900Changes from 17 commits
d785d2b
cfd9111
7593fa9
6065c88
a4630d0
071c277
1a5cf7c
4460a7e
695efdf
c74afe5
b55daaa
2429fe8
4cf94a5
8e957bd
37e1178
ef5ac81
653c90a
7adf081
56c2eed
f2595e9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think the wording is unclear here for someone not familiar with the design proposal.
What does image "image top" and "text top" mean here? Better wording would be Align this section of the formatted string so that the top of this section is in line with the top of the other sections. And similarly for the other descriptions.
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.
Good point, done. I simplified the text you've proposed a bit:
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.
The example should be updated to include a
vertical-align
section.Ideally we would include the images from the design proposal.
If that is difficult for some reason, please update the design proposal with the latest modifications (i.e. use
bottom
instead ofbaseline
and link to it so people have some additional context and visuals.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 have updated the example:
I also referenced design proposal. It is updated to use
bottom
instead ofbaseline
.For the images - technically I can do it (it works) however there are no images in Expression documentation at all. I'm not sure what is a pattern here. Also I think it is good that expressions documentation is concise, and I'm not sure if adding images won't change that. I think it will be the most valuable to add or update example after maplibre-gl-js implementation. What do you think?