Skip to content

Conversation

@ianjon3s
Copy link
Contributor

@ianjon3s ianjon3s commented Oct 17, 2024

What does this PR do?

Improves visibility of point labels by removing character stroke and keeping background/text colour constant (not dependent on custom colour selection).

Preview (before vs after):
image

@netlify
Copy link

netlify bot commented Oct 17, 2024

Deploy Preview for oslmap ready!

Name Link
🔨 Latest commit e28c6c8
🔍 Latest deploy log https://app.netlify.com/sites/oslmap/deploys/67178b432e372700082c462b
😎 Deploy Preview https://deploy-preview-507--oslmap.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ianjon3s ianjon3s marked this pull request as ready for review October 17, 2024 14:57
@ianjon3s ianjon3s requested a review from a team October 17, 2024 14:57
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi ! I think this looks great for drawMany cases when labels are visible - but can we please confirm this is still the intended style for:

  • drawing a single point
  • drawMany with hideDrawLabels as recently introduced in #508

I think I'd still expect to still see a solid colored point in these cases rather than the stroke only?

Screenshot from 2024-10-21 10-26-25

@ianjon3s ianjon3s force-pushed the ian/dot-marker-label-contrast-improve branch from 13ae4b1 to e28c6c8 Compare October 22, 2024 11:23
@ianjon3s
Copy link
Contributor Author

Good point @jessicamcinchak , however we still want to show two contrasting colours to safeguard against any contrast issues with the background map (green on green < green+white on green). I've updated the stroke to be thicker in the absence of labels:

image

Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect thanks for update!

@jessicamcinchak jessicamcinchak merged commit f938a93 into main Oct 23, 2024
@jessicamcinchak jessicamcinchak deleted the ian/dot-marker-label-contrast-improve branch October 23, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants