Shadows #86
Shadows #86
Conversation
@rollingversions/server (2.14.0 → 2.15.0)New Features
Packages With No ChangesThe following packages have no user facing changes, so won't be released:
|
| export function InstallButton({ | ||
| size = 'sm', | ||
| shadow = 'gray', | ||
| }: { | ||
| size?: 'sm' | 'lg'; | ||
| shadow?: 'gray' | 'orange' | 'white'; | ||
| }) { |
There was a problem hiding this comment.
Constructing class names like this will break in production. Just accept a className?: string prop and add that into the class list.
The issue is that there's a production optimisation that checks each class name in the CSS to see if it's used in code, and removes the un-used css classes. It's not super smart, so it's just looking for occurrences of that string in the JavaScript code that's output, so it wont' be able to connect focus:shadow-${shadow} with the fact that you pass in shadow somewhere else.
There was a problem hiding this comment.
updated, but I've gone with setting shadow options to 'shadow-gray' rather than just 'gray' etc. This is because I want to default the shadow to 'shadow-gray' and className doesn't feel like it should just be defaulted if empty. Is this ok?
| return ( | ||
| <a | ||
| href="https://github.com/apps/rollingversions/installations/new" | ||
| className={`flex items-center justify-center bg-black text-white italic font-poppins font-black ${ | ||
| className={`flex items-center justify-center bg-black text-white italic font-poppins font-black focus:outline-none focus:${shadow} ${ |
There was a problem hiding this comment.
The optimiser will not be able to look at focus:${shadow} and separately shadow="shadow-gray" and figure out that the focus:shadow-gray class is needed. You need the entire string focus:shadow-gray to appear somewhere. Since this is just a class name, the normal pattern is to allow the component to accept additional classes via an optional className property. The alternative is to have modes like shadow?: 'gray' | 'orange' | 'white' and then actually split them out to generate the classes via something like:
const shadowClasses = {
gray: 'focus:shadow-gray',
orange: 'focus:shadow-orange',
white: 'focus:shadow-white',
};The important thing is that the entire focus:shadow-x string needs to appear somewhere in the code.
No description provided.