feat: Tree node styling. #592

dhess · 2022-10-26T14:16:10Z

This series of commits attempts to bring our current tree implementation closer to the UI design spec.

A few notes: * Our Tailwind config inverts the blue primary and secondary colors wrt. the document. (By my recollection, I think this was intentional and had something to do with the corresponding hover colors.) * Ann's document lists "red secondary" twice, once as #cd3764 and once as #f1685e. I believe #cd3764 was added late in the process and therefore I've called it "red tertiary" in this commit. * Ditto for "grey secondary," but in this case, the hex RGB value given for the 2nd occurrence of "grey secondary" does not match the color swatch. The hex value is the same as the 1st occurrence of "grey secondary" and therefore I've used the hex RGB value I measured by sampling the swatch in the document (which is #d4d4d4). I've called this "grey quaternary" in the Tailwind config.

This will become more useful in the next couple of commits.

cloudflare-workers-and-pages · 2022-10-26T14:16:44Z

Deploying with Cloudflare Pages

Latest commit:	`986024d`
Status:	✅ Deploy successful!
Preview URL:	https://0eae140b.primer-app.pages.dev
Branch Preview URL:	https://dhess-style-nodes.primer-app.pages.dev

View logs

dhess · 2022-10-26T14:17:06Z

I'm not going overboard here because we are likely to toss the current tree implementation, but I'd like to get closer to the design spec for our upcoming testing.

We don't use all of these at the moment, but it's easy to forget these are necessary, so we might as well define them in advance.

("Mostly" because some of these nodes should be filled a solid color with white text.) The summary commit message describes the only behavioral change in this commit, but I've buried the lede: this commit also majorly refactors how we assign classes to nodes & edges in the tree by using Tailwind CSS classes, rather than injecting raw `style` CSS attributes. At the moment, this doesn't buy us much and instead feels a bit like a violation of DRY, but it does give us a lot more flexibility in styling, which the next few commits will take advantage of.

The following flavors aren't selectable, and therefore we don't show hover for them: * `with` in `match ... with`. * Pattern boxes. * Pattern constructors. * Pattern applications. (Notably, pattern binds are selectable because we can rename them.) This implementation deviates from the design spec by making the width of the outline 4px, rather than 2px. I think 2px is a bit too thin, at least for our current overall style.

Per the Tailwind CSS docs, this is a 6px radius, which is what the UI design spec calls for.

I had been wondering separately what was the difference between `ring` and `outline` and finally stumbled across an explanation of why `ring` was added in the first place: tailwindlabs/tailwindcss#7649 It turns out that on Safari, outlines don't follow the border of the element they're outlining: they are always squared off. Tailwind CSS addressed this by adding `ring`, so for our purposes, this is what we should be using.

There's still some work to be done here, but this is a good approximation of the design spec. Among the things that remain to be done are: * We don't draw any labels on most syntax nodes. * The size of the label and the contrast need some work. * The single-glyph labels render like relatively flat ellipses rather than nice circles like the 3-glyph labels do. We might be able to fake this by padding out the single-glyph labels to 3 (spaces on either side), or perhaps with a bit of min-width magic.

With this commit, we pretty closely match the UI design spec for tree styling, with a few exceptions: * Holes are not rendered as a circle. * We don't have an `=` node for `let`s. * Value constructor and function `apply` nodes read just `apply` rather than `apply function`. We need to think about this a bit in the context of patterns, where `apply` only ever appears with a value constructor and we don't want students to get confused. Note that there are a few node types that weren't specified in the UI design spec (e.g., type constructors). At the moment, these are rendered in black, and we'll need to choose some colors for these. Also, nodes that appear in patterns are also rendered in black. I'm guessing this is a leftover from a debate we had a few months ago about whether patterns should be treated specially other than the box drawn around them. This will be corrected in a subsequent commit. Finally, the body content I've chosen here for some syntax nodes (e.g., `function type`) should be considered just placeholders for now. We should finalize the terms we use for these at a later date. (One complication is that they should be reasonably short, so that they don't take up too much space in the tree.)

dhess · 2022-10-27T02:08:29Z

This is pretty close to the final design for expression trees (ignoring the changes we've made to edges). Off the top of my head, we still need:

Round hole nodes, but I would like to try implementing "shrink-to-fit" node rendering first.
Decisions on what to render for syntax nodes like annotations and function type constructors.
Color choices for node types that don't show up in Ann's UI design spec, such as type constructors and variables.

2 and 3 will probably require some bikeshedding and 1 will take me some time to figure out. Therefore, I think this is ready for review.

(And of course, this PR doesn't do anything about rendering type trees or top-level definition names.)

Note: yes, there is quite a bit of repetitive code in the Tailwind class functions. We should definitely take advantage of some default cases in the switch statements to DRY, but at the moment this is nice because it's easy to make one-off changes to special nodes. Once we have a reasonably final collection of class functions, I will take a pass at batching similar node styles together.

dhess · 2022-10-27T02:10:17Z

BTW, there are a few things that I don't like about the design spec now that we've implemented most of it in a real application. For example, I don't think the green selection ring works well. I will compile a list of these when I get a chance, and we can discuss some alternatives. I encourage everyone to do the same after you've had a chance to play with the changes in this PR.

Previously they were rendered in black.

dhess · 2022-10-27T09:44:50Z

In this PR, there's one notable deviation from the UI design spec. I've placed the "P" label for patterns at the midway point of the top edge, rather than in the upper right corner as the spec calls for.

This PR:

The spec:

I did that because other "syntax" nodes like match and apply have the centered label, and I thought it might be good to be consistent about that. However, patterns are different in that students can modify them. At the moment, we permit the student to rename a pattern variable, and someday we'll give them the ability to write their own patterns. So in that sense, they're not like the other syntax nodes, and an argument could be made for putting the "P" in the upper right-hand corner like other student-definable nodes such as names and bindings.

(Hmm, it now occurs to me that maybe we should treat constructors as syntax nodes?)

georgefst · 2022-10-27T10:03:38Z