Use FontFace and consistent text sizing #988
Conversation
boatbomber
added
scope: plugin
|
|
||
| local params = Instance.new("GetTextBoundsParams") | ||
|
|
||
| local function getTextBounds(text: string, font: Font, textSize: number, width: number, richText: boolean?): Vector2 |
There was a problem hiding this comment.
This function can yield, so it should probably have a name that reflects this - and it's also not OK to yield in Roact! I'm not sure exactly when it will yield, and our TextService:GetTextBoundsAsync usage has seemed fine so far, but it probably warrants further review. You're more familiar with this area of the codebase so you'll be more able to tell if it's a problem or not
There was a problem hiding this comment.
GetTextBoundsAsync only yields when the font isn't loaded, as per Roblox's own docs. Assuming we trust them, we could ensure it never yields by preloading the fonts when the plugin initializes.
That said, I suspect that builtin fonts are just always loaded in Studio or load quick enough that it won't matter. This would be more of a concern if we were using a custom font.
| local params = Instance.new("GetTextBoundsParams") | ||
|
|
||
| local function getTextBounds(text: string, font: Font, textSize: number, width: number, richText: boolean?): Vector2 | ||
| params.Text = text |
There was a problem hiding this comment.
This property set should probably have a check for text with length >= 200000, right now it's possible to brick the entire Rojo UI by trying to view an extremely large script in the diff viewer
The
Gothamfont we use no longer exists. This PR upgrades to usingFontFace, and also unifies our styling by putting fonts and text sizes intoThemeinstead of hardcoding them inline. This will make it easier to adjust our UI in the future (or perhaps add accessibility settings).It's pretty much identical to before, but all the text sizes are consistent (no more 14 vs 15) and the notifs and tooltips are better at getting the correct text bounds (no cutting off edge cases anymore).