[Fixes #34] Add setTagStyle() and deleteTagStyle()

[bluepichu]

Current Revision

• setTagStyle(tag: string, style: ExtendedTextStyle): void updates the given text style by overwriting the specified fields in the existing style object with the new given values.
• deleteTagStyle(tag: string): void deletes the styles for the given tags. If the tag is default, then the default styles are reset to their defaults.

Original

I fixed this as discussed, but changed the method name slightly and added two other helpers:

• setTagStyle(tag: string, style: ExtendedTextStyle): void sets the text style object for the given tag to a copy of the given object. If the tag is default, then it also adds the default values if they are missing.
• updateTagStyle(tag: string, style: ExtendedTextStyle): void updates the given text style by overwriting the specified fields in the existing style object with the new given values.
• deleteTagStyle(tag: string): void deletes the styles for the given tags. If the tag is default, then the default styles are reset to their defaults.

[tleunen]

Hmm... Not sure I follow the reasoning behind both set and update. I'd rename this one as set and remove the old one. And I'd replace the content for something like this

if ({}.hasOwnProperty.call(this.textStyles, tag)) {
  this.assign(this.textStyles[tag], MultiStyleText.DEFAULT_TAG_STYLE, this.textStyles[tag], style);
}

[bluepichu]

My thought was to include set as something of a shorthand for delete and then update, for if you want to entirely overwrite the style. In particular, I thought that update would be the more common use case, but calling it set might give the wrong idea of what is being done to the style object.

Regarding the content replacement:

• Could you explain what the functional difference between hasOwnProperty and in would be in this case? Since this.textStyles shouldn't have a prototype chain, the usual distinction between them doesn't seem to me like it should matter.

• If hasOwnProperty is the way to go, why {}.hasOwnProperty.call(this.textStyles, tag) instead of this.textStyles.hasOwnProperty(tag)?

• The internal this.assign() I believe is correct as-is. Remember that we no longer set all of the text properties on each tag so that we can use assign to do inheritance when nesting tags.

[tleunen]

I believe having 2 methods to "change" a style will confuse people.

For the comments, sure :)

• You're right. Acually we could also just do if (this.textStyles[tag]) then :)
• That way you're sure to use the proper hasOwnProperty function, not a potential one overriden by the object
• Actually I'd like to have Babel handle the Object.assign polyfill, and anyway I believe it's safe to use Object.assign since it's available everywhere, except IE9 maybe but does Pixi still support IE9?

[bluepichu]

Sorry for the delay, got busy for a while there.

I renamed updateTagStyle to setTagStyle and removed the old one. Everything else was left the same.

If we want to replace the assign fill, I think we should do that in a new PR.

[ArronFerguson]

Cool, awesome, thank you so much for looking into this! Is this in the source in a file that is available? I did a pull (npm install pixi-multistyle-text) but didn't see the changes in it.

[bluepichu]

Currently the only way to get a build of this would be to checkout the branch bluepichu:iss34-es5 and then run npm install and npm run build.

[ArronFerguson]

Sorry, I'm not so up on GitHub, I went to:

https://github.com/tleunen/pixi-multistyle-text/pull/35/files

And saw the ".ts" file, and downloaded it, renamed to ".js". I noticed some of the JavaScript 6 reserved words in there (e.g., export), which aren't yet supported in any of the browsers. Am I looking at the correct file and if so, is there a JS 5 version?

[bluepichu]

@ArronFerguson, try this (updated):

git clone https://github.com/bluepichu/pixi-multistyle-text.git
cd pixi-multistyle-text
git checkout iss34-es5
npm install
npm run demo

If the demo page opens in your browser, the build succeeded. The built JS file is located at dist/pixi-multistyle-text.js.

@tleunen, do you have time to look at this sometime soon? If not, I'd like to merge it and push out a new release so that people can use this.

[ArronFerguson]

Matthew,

Thanks for the response. I got:

Cloning into 'pixi-multistyle-text'...
Warning: Permanently added the RSA host key for IP address '192.30.253.113' to the list of known hosts.
Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

I'm in MINGW64 terminal. Is this a repository access restriction or MINGW64 misbehaving?

[bluepichu]

Sorry, I goofed and gave you the wrong address (SSH instead of HTTPS). I updated the instructions above; try it now.

[ArronFerguson]

Got it! Thank you so much Matthew! Thanks for taking the time to do this, I really appreciate it. Just playing with shaders now on my site and trying to make it all responsive - which was the reason why I wanted the ability to change style in the first place.