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.

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 lineWidth support to RichText #998

Closed
zepumph opened this issue Sep 9, 2019 · 5 comments
Closed

Add lineWidth support to RichText #998

zepumph opened this issue Sep 9, 2019 · 5 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 9, 2019

After investigating on my own in phetsims/inverse-square-law-common#79, and then discussing with @jonathanolson, it looks like RichText doesn't currently support lineWidth. We decided to add it almost identically to how the stroke feature is implemented. I'll take a look.

@zepumph
Copy link
Member Author

zepumph commented Sep 9, 2019

A few notes for the review

  • lineWidth currently isn't supported with underline or strikethrough. That felt alright to me, as it doesn't look like stroke is supported there either.
  • lineWidth wasn't added to RichTextLeaf.clean because it doesn't hold any references
  • There are pretty bad artifacts on the tops of the letters after adding lineWidth to ISLC sims over in Updates to ISLCObjectNode's label text inverse-square-law-common#79. @jonathanolson do you know what the deal is there? I don't think I can publish those sims, or any using RichText.lineWidth until this is resolved. UPDATE: It seems like when you resize the sim they go away. Testing on windows and latest chrome.

@jonathanolson please review.

@jonathanolson
Copy link
Contributor

I'm not seeing artifacts on Chrome/macOS. Can you take a screenshot or note reproduction instructions? I'm also not sure there is anything we can do in that case. (Can you also try ?rootRenderer=canvas and ?rootRenderer=svg to see if it makes the artifacts go away?

Main commit looks great!

@zepumph
Copy link
Member Author

zepumph commented Sep 10, 2019

image

It did not help to change the rootRenderer. Is this sort of thing familiar to you?

@pixelzoom
Copy link
Contributor

Imo, stroking a font (in general) looks lousy. You'd be better off filling bold text with a color that looks good on each sphere, or putting a translucent background behind filled text.

@zepumph
Copy link
Member Author

zepumph commented Sep 12, 2019

Imo, stroking a font (in general) looks lousy. You'd be better off filling bold text with a color that looks good on each sphere, or putting a translucent background behind filled text.

That sounds good. I will look into those options over in phetsims/inverse-square-law-common#79.

I'm also not sure there is anything we can do in that case.

I don't know what else there would be to do. This artifact only occurs with a very thin lineWidth, and not on all browsers. I feel alright about leaving it up to the implementer to choose a format that looks good, and doesn't have these artifacts.

Closing

@zepumph zepumph closed this as completed Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants