Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Rendering tweaks #62

Merged
merged 9 commits into from
Apr 6, 2021
Merged

Rendering tweaks #62

merged 9 commits into from
Apr 6, 2021

Conversation

binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented Mar 10, 2021 •
edited
Loading

This PR makes the following changes:

  • Allows specifying font families and font sizes for segment labels and place labels in the styles property,
  • Render the background of segment labels with a size consistent with the font size specified in the styles property (a method that consistently formats the font size string is added behind the scenes),
  • Fix lines have spikes (specially in higher map zool level) when there are many segments  #58 (jagged edges with spikes),
  • Updated Storybook stories.

@binh-dam-ibigroup binh-dam-ibigroup changed the title Transitive labels Rendering tweaks Mar 10, 2021
@binh-dam-ibigroup binh-dam-ibigroup marked this pull request as ready for review March 11, 2021 23:02
evansiroky
evansiroky reviewed Mar 23, 2021
View reviewed changes
lib/util/index.js Outdated
@@ -2,6 +2,7 @@
* General Transitive utilities library
*/

import isNumeric from 'isnumeric/isNumeric'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a huge fan of adding a whole new package just for this little use here. I'd just go ahead and try to inline some kind of logic in this file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed the explicit reference in 7e65ad5.

Interestingly, isnumeric is implicitly referenced by another library (one of CSS libraries it looks like).

evansiroky
evansiroky suggested changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@evansiroky evansiroky left a comment

Choose a reason for hiding this comment

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

See comment

@binh-dam-ibigroup binh-dam-ibigroup removed their assignment Mar 29, 2021
@evansiroky evansiroky removed their assignment Mar 29, 2021
landonreed
landonreed approved these changes Apr 5, 2021
View reviewed changes
Copy link
Contributor

@landonreed landonreed left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. This looks good. See recommendation regarding the new story's name. Also, I noticed that changing the segment label size to 0px makes things crash. Not sure if that's from these changes or something else in transitive?

stories/Transitive.stories.js Outdated Show resolved Hide resolved
@landonreed landonreed merged commit 450fb2a into conveyal:master Apr 6, 2021
@landonreed
Copy link
Contributor

🎉 This PR is included in version 0.13.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@landonreed landonreed landonreed approved these changes

@evansiroky evansiroky evansiroky approved these changes

Assignees

@landonreed landonreed

Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lines have spikes (specially in higher map zool level) when there are many segments
3 participants
@binh-dam-ibigroup @landonreed @evansiroky