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

Say/think bubble SVG templates #177

Merged
merged 3 commits into from
Oct 11, 2017

Conversation

paulkaplan
Copy link
Contributor

@paulkaplan paulkaplan commented Oct 4, 2017

Proposed Changes

Describe what this Pull Request does

This PR builds on #176 (the first commit here) and two parts:

  1. A utility for generating say/think bubble svg strings.
  2. A public API for creating skins using the say/think bubble svg templates by passing in the text to be displayed and some other required info.

This is meant as conversation-starter code, it is still very early. The things I want to talk about w/ @cwillisf are:

  • This code hijacks the svg-renderer in order to measure the width/height of the text block before creating the enveloping path. The svg-renderer isn't meant for this, because it also draws it to a canvas which we do not need. Is there a better way to share the measurement code, possibly drawing out and expanding the SVGMeasurementProvider used in the text wrapping code?
  • This code exposes new methods for creating/update "text skins", but there is no TextSkin subclass, it is really just an SVG skin. Is that an ok way of doing it?

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

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

I left a comment on #176; the comments here are only on the second commit.

return `<text fill="#575E75">${this.lines.join('\n')}</text>`;
}

toString (type, text, pointsLeft) {

This comment was marked as abuse.

This comment was marked as abuse.

if (!this._textSizeCache[svgString]) {
this._textSizeCache[svgString] = this.svgRenderer.measure(svgString);
}
return this._textSizeCache[svgString];

This comment was marked as abuse.

This comment was marked as abuse.

@cwillisf cwillisf removed their assignment Oct 11, 2017
@paulkaplan paulkaplan merged commit b84721d into scratchfoundation:develop Oct 11, 2017
@paulkaplan paulkaplan deleted the say-think branch October 11, 2017 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants