PB-266: Corrected KML icons size #641
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Passing run #588 ↗︎
Details:
Review all test suite changes for PR #641 ↗︎ |
|||||||||||||||
ltshb
force-pushed
the
feat-PB-266-icon-size
branch
5 times, most recently
from
3be761a to
b685964
Compare
ltshb
force-pushed
the
feat-PB-266-icon-size
branch
2 times, most recently
from
1b2ec8d to
b0a3d6a
Compare
ltshb
force-pushed
the
feat-PB-266-icon-size
branch
from
b0a3d6a to
c365fa8
Compare
ltshb
force-pushed
the
feat-PB-266-icon-size
branch
from
c365fa8 to
0d85eaa
Compare
ltshb
force-pushed
the
feat-PB-266-icon-size
branch
4 times, most recently
from
d6a292b to
2ef6735
Compare
pakb
reviewed
Feb 14, 2024
| * @returns {String} A full URL to this icon on the service-icons backend | ||
| */ | ||
| generateURL(iconSize = MEDIUM, iconColor = RED) { | ||
| generateURL(iconColor = RED, iconScale = 1) { |
There was a problem hiding this comment.
why not using a constant anymore? (SMALL, MEDIUM, etc...)
There was a problem hiding this comment.
Because the icon scale of MEDIUM, ... is the open layer scale and here I want to be sure to get the icon at its scale 1 by default and we always takes icon from backend at the same scale and let openlayer do the scaling.
src/utils/kmlUtils.js
Outdated
| // scale_factor := iconStyle.getSize()[0] / 32 | ||
| // iconStyle.getSize()[0] = 48 (always 48 pixel on old viewer) | ||
| // scale_factor = 48/32 => 1.5 | ||
| iconStyle.setScale(iconStyle.getScale() * 1.5) |
There was a problem hiding this comment.
it might be good to move the constant ICON_XML_SCALE_FACTOR declared in E2E test here, export it, and re-use it in the test too.
ltshb
force-pushed
the
feat-PB-266-icon-size
branch
from
2ef6735 to
febbeec
Compare
pakb
approved these changes
Feb 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Now the KML icon size and text size is retro compatible with the old kml adding a new
extra largesize.The icon scaling is a bit tricky due to a factor to normalize icons to 32 pixel introduced by open layer in version 6.7 (old viewer used an older openlayer version, see openlayers/openlayers#12695).
Relates to #628
Test link