Move toward a "black box" component library model for `packages/ui`

[jessepinho]

At the moment, our @penumbra-zone/ui package contains components whose appearance can be highly customized by the devs using them, particularly through the use of the className prop.

Ideally, a component library should be much less customizable. In the same way that an API exposes specific endpoints rather than allow any client to make SQL queries directly against its database, a component library should have a specific set of "endpoints" (in the form of component props) that allow them to be customized in pre-defined, rather than ad-hoc, ways.

The idea is that developers shouldn't generally be making design decisions when working on a given UI ticket, as that leads to poor UX decision-making. Designers are the ones responsible for making sure that an app has visual unity, and that each user flow is optimized to make it as painless and intuitive as possible. Developers, meanwhile, are generally focused on making a feature work, even if it comes at the expense of those two design concerns. Thus, if the design of an app is led by developers, its visual unity will gradually disintegrate over time, as each developer tweaks designs to their own tastes. And, common workflows will often suffer without a designer's research and expertise.

Thus, I am suggesting the following:

1. Develop a component library + design system in e.g., Figma.
   • This should include enough components that nearly all UI tickets can be completed by combining existing components, rather than creating new ones.
   • The system should be extremely specific. For example:
     • It should specify base layout and spacing units (such as 4px for spacing and e.g., a 12-column grid).
     • Each component should have its internal spatial relationships defined. (External spatial relationships are always determined by the container, not be the component itself. e.g., a button should never have external margins, as those should be set by their containing elements to avoid CSS override hell; but it is permitted to have internal padding.)
   • The system should specify shared values like border radii, brand colors, typefaces, etc. along with what each is used for.
2. Implement this component library in @penumbra-zone/ui.
   • Customization of these components, beyond what is specified in the Figma designs, should be disabled. For example, if Figma calls for a button with several "variants" like primary, secondary, and icon — each of which have different background colors and sizes — these can be specified via the <Button /> component's variant prop, NOT via a className prop that allows customization of colors and sizes.
   • Most, if not all, of these components should be built from scratch, rather than on top of an existing UI component library. In my experience with component libraries (both off-the-shelf ones, as well as the custom one I implemented from Figma designs for my previous employer), trying to build brand-compliant components on top of existing UI component libraries is unnecessarily painful, with tons of overrides/etc. required to get components to look how they should. It's much simpler and cleaner to build these components from the ground up. The only exception to this rule is when complex accessibility and/or interactivity concerns are at play, such as with a floating dropdown menu.
3. Sunset our use of Tailwind.
   • Tailwind is designed to make it easy for developers to customize the appearance of components — AKA, exactly what I'm suggesting we don't want.
   • Tailwind classes make it hard to reason about what styles a given element has, as it forces developers to inline all styles for elements, usually in a single line. For example: https://github.com/penumbra-zone/web/blob/main/packages/ui/components/ui/dialog.tsx#L40
   • Instead, we should use Dripsy, an unopinionated library that makes it easy to provide various styles to components via React contexts. (I've had success using this for building a ground-up component library in the past.)

[jessepinho]

cc @grod220 per our discussion on the topic

[grod220]

Helpful! A few thoughts:

• There will be some sizable design changes coming down the pipeline in the next few months. We likely should put off implementation of this until we have the delivered figma w/ new brand guidelines. It is also likely we'll have two brands: penumbra main and a sub-brand for the extension. This is meant to distance the extension a bit from being considered the canonical one.
• The className prop is really a kind of inversion-of-control that shadcn/ui offers. It's meant to replace the need to A) wrap the component in a div w/ custom styling (like wrapping a div with position: relative) or b) add a ton of optional props for one off styling. However, in theory, one could pass a ton of styling that really should live in the UI component. So it could be abused. Worth trying to be stricter, however, if folks are constantly wrapping it, that may be an indication that className props are necessary.
• Re: building all components from scratch -> I find Kent Dodd's reasoning compelling. The industry is moving toward headless component libraries. The idea is to use a UI library with first-class accessibility, but that does not come with any styles. You bring your own. Shadcn/ui took this idea further and made it easier for tailwind customization of Radix components. If the component exists there already, I don't see a reason why we wouldn't use it as a starting point.
• Deprecating Tailwind is a tough one to swallow. Doing so is kinda a bet against the industry. The adoption and support for it has grown substantially since it came out. The syntax definitely takes some time to get used to, but quickly becomes way faster/easier for styling. I's say Theo's journey with tailwind really echoes mine. It's not just for our UI components though, we will need to style things ad hoc anyway.

[jessepinho]

@grod220:

There will be some sizable design changes coming down the pipeline...

Yup, makes sense.

The className prop is really a kind of inversion-of-control…

The best way to avoid pain here is: anything that affects the internal elements of a component should be handled by the component itself (with the optional input of limited props like variant). Anything affecting the external relationships between the component and its surroundings should be handled by the container component.

The reason I don’t like e.g., passing position: relative directly to a component’s root element is that it can interfere with a component’s internal styling logic. Let’s say, for example, that we have a component with rounded corners. And let’s say you want to make one specific instance of that button’s have smaller corner radiuses than the rest of the buttons, since it’s going to be a small button where larger radiuses would look weird. So you pass a rounded-sm class to it, which gets applied to the root element. Later, we get new brand guidelines that all of our buttons should have square corners, not rounded. We update the component to have squared corners. Now, all our buttons are squared, except for the one you passed rounded-sm to. No one realizes this, because there’s no type checking or anything that ensures that we no longer round the corners of our buttons.

If we had instead created e.g., a size prop on the <Button /> component, the <Button /> would have its own internal logic to use smaller corner radiuses when size === ‘small’. (For that matter, the developer who created the <Button /> component would hopefully have been thoughtful about what exact radius to use for each size, to fit together aesthetically.) Then, when we got updated brand guidelines, we’d remove any use of border radiuses from inside the <Button /> component, and buttons would still look consistent throughout the app. That way, the domain-specific knowledge about how to render a <Button /> stays within the <Button />, rather than being distributed in random places throughout the app.

The idea is to have clear boundaries between parent and child components, in terms of styling. If a parent can modify a child’s classes, the child then has to be aware of every possible style modification that can be made to it. But if a parent can only pass in a limited set of props, the child can control exactly what it renders for every combination of props.

…in theory, one could pass a ton of styling that really should live in the UI component. So it could be abused.

I would argue that Tailwind is designed to encourage that kind of abuse.*

The industry is moving toward headless component libraries.

Yeah, I can definitely see the benefits of this — especially to your point about accessibility. I just read his post that you linked, and I hadn’t realized that ShadCN is a kind of copy-paste solution. Looks pretty cool.

Deprecating Tailwind is a tough one to swallow.

I'm open to being wrong on this. I'll take some time to study up on Tailwind so that I understand its philosophy from the inside out, rather than just judging it by its cover. (I watched the video of Theo's journey, though: while his skepticism was similar to mine, I didn’t find his conclusions particularly compelling. To be honest, it feels a bit like Tailwind was designed for people who dislike CSS, which, I mean, fair… but it’s what we have to work with, and it’s worth doing right.)

Re: it being a bet against the industry: I wouldn’t necessarily assume that its popularity means that it’s a good or responsible solution for the long-term maintenance of a web app. It’s equally possible that its popularity stems from people wanting to not have to switch between TSX and CSS files when coding, which isn’t necessarily a good reason for its adoption. That said, again, I’ll take some time to study up on Tailwind so I’m more informed on why it’s so popular.

* Big caveat to pretty much all of my arguments above: I think I’m more accustomed to working with meticulously crafted design systems in Figma, and then implementing them in code. In that context, you really don’t want developers to be making design decisions. Since we don’t currently have a super built-out design system in place, allowing more design-by-developers is the only option we have, and Tailwind certainly makes that a lot easier. (Tangentially: I might also be conflating two concerns: 1) deprecating Tailwind, and 2) keeping clearer boundaries between parent and child components. Will reflect on this more…)

[VanishMax]

"Black box" model was described in #1406, followed by #1411 and implemented later. Closing as done

The future plans of UI package development are moved to #1710