Fix CircularOutline performance

[joshwcomeau]

Related Issue: closes #225

Summary:
Our component triggered an immediate re-render after mount, since it needed to read the SVG created by React. This was causing performance issues (Stuttery modal opens, mostly). It was also causing a weird issue where the outlines would disappear sometimes, due to a Chrome bug (most likely).

This change creates a new division between two types of outlines:

• Truly circular ones, like the ones used in ProjectIcon. Round circles with an outline
• Outlined buttons, which are really just rounded rectangles

For the former, the behaviour is unchanged. This is possible because we know the width/height of those icons, so we can use geometry to work out the path length instead of relying on the DOM.

While it may be possible to do the same for rounded rectangles, we don't currently have any way of knowing the width of buttons, and because our buttons can change widths dynamically, it felt like too much trouble. Instead, I went for a new approach: using a background gradient.

Essentially now the StrokeButtons have a "background" layer, and a "foreground" layer that holds the actual button. The background layer has a solid background gradient, but the foreground layer has a background colour that overrides it, and some padding so that the background layer is larger. Because of this, it appears to just be an outline.

The downside is there's no clear way to do the "self-drawing" effect on click... but I don't feel too sad about it. Instead I added a simple fade in/out. The only place that uses this effect in the app is the project-type selection in the new project wizard, and I actually prefer the new effect here, it's less "busy":

(oh, also the random generator button uses it too, but I want to replace it anyway. Created a new issue, #264)

Profile diff:

Here's the difference in speed when opening the "Project configuration" modal:

Before:

After:

This makes the modal feel like it opens more quickly. I imagine the difference is even more stark on less-powerful machines (I work from an iMac Pro)

GIFs:

Here's the storybook for the new stories:

[joshwcomeau]

Both StrokeButton and SelectableItem needed the same hover behaviours, and because events propagate, I was able to create its own component for it.

[mathieudutour]

I know this is a bit unrelated but how do you feel about changing color1 and color2 to colors so that it matches the API of the other Buttons?

[joshwcomeau]

The only issue is that because it's an SVG gradient, it's not as trivial to set up (not that it's super hard, but you'd need to add stops for each color provided, and work out the percentages for each stop)

I could use Flow to enforce that it's a tuple of 2 colors, so that the API matches but it's a limited number of entries... but I'm not sure how many folks run Flow in realtime, so it might not help them if they're confused why the third color isn't showing up. Although I don't expect us to ever need 3 colors for circular outlines anyway, so maybe that's not a real concern?

[mathieudutour]

Don't this it would be too hard to support an arbitrary number of stops: #266

[joshwcomeau]

Right, so I didn't mean that it would be genuinely challenging, just that it's maybe not worth the time given that we have no need of 3+ color gradients.

Your PR looks like it'd work to me, but it also adds quite a bit of complexity to the code - it's a bit harder to follow with a dynamic number of stops. I'm not sure how it handles edge cases either - for example, if the colors I supply are ['#FFF'], I won't get a solid white edge, but a gradient from white to purple.

I think if we wanted to go this route, we should add an SvgGradient component which can handle any number of colors, add some stories for it (well, timing's a bit awkward as we move to docz) to make sure it handles edge-cases.

I think we should create a new issue for it, with the "Good first issue" tag. Maybe someone would be interested in picking it up?

[mathieudutour]

Pretty sure it’s handling [‘#FFF’] as it should (taking the latest color stop for 100% which would be white as well) but I see what you mean

[joshwcomeau]

Pretty sure it’s handling [‘#FFF’] as it should (taking the latest color stop for 100% which would be white as well) but I see what you mean

Ah, you're right! Sorry, probably should have actually tried the code before commenting. Tried to get a reply out quickly between tasks at work.

I do like the idea of a consistent API, and it's probably worth the few minutes testing your PR + updating existing callsites. I'll try and get this done tomorrow morning, although I'm not sure how much time I'll have. I'll also create a Good First Issue ticket to extract a gradient component with proper stories and such

[joshwcomeau]

@mathieudutour I merged your PR into this one, and did some refactoring to (hopefully) make it easier for newer devs to understand. Updated the stories as well.

There's no more color1 or color2 anywhere in the codebase now :D it does feel really good to have a consistent API.

If you have the chance, would love your feedback. No worries if not though.

[joshwcomeau]

Actually @mathieudutour, I really wanna get this merged so that it (potentially) unblocks #213, but feel free to add comments anyway, I can always address them in a followup PR.

[mathieudutour]

Nice comments 👍