button-component #29
jare2473
ansv7779
Loading…
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "button-component"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Button component.
Addressing issues #15 and #16
Can be viewed on:
https://studentportalen-button-component.branch.dsv.su.se/
Feedback:
cursor: pointer;-css on hoverLooks good! I will make icon-only button as a separate component and issue.
Functionally it looks good and does what one would expect when using it.
Couple of points about code clarity.
I also see we need to add a workflow for Prettier checking.
@ -0,0 +11,4 @@const variantClasses: Record<ButtonVariant, string> = {primary:"bg-primary text-base-canvas border border-primary hover:bg-secondary hover:text-base-canvas hover:border hover:border-primary focus-visible:bg-base-canvas focus-visible:text-base-ink-strong focus-visible:border focus-visible:border-primary focus-visible:outline focus-visible:outline-[length:var(--border-width-lg)] focus-visible:outline-sky-100",This is unwieldy. Any way to break it down or group them somehow? Like the border styles, focus styles, colors, and so on.
Is it necessary to repeat the prefix or can you do something like
hover:[a, b, c]instead ofhover:a hover:b hover:c?@ -0,0 +46,4 @@sizeClasses[size],className,].filter(Boolean)What does this do? I assume it does some JavaScript type-coercion of strings/
undefined/nullto booleans but what about the empty string? Needs a clarification comment.My initial read through of the code, none of the strings in the list should be
nullorundefinedand joining an empty string shouldn't matter much.You're right that the current implementation passes only non-empty strings, so
.filter(Boolean)doesn't change behavior today.The reason it's useful is as soon as someone adds conditional classes or optional props (e.g.
isDisabled && "opacity-50 cursor-not-allowed"), it's easy for the array to end up containing false, undefined, or empty strings. Without filtering, those values can end up in the final className and produce a noisy DOM.Using
.filter(Boolean)keeps the class string clean no matter how the component evolves. I'll add a short comment explaining this so future contributors understand the intent.@ -1,11 +1,259 @@@import "tailwindcss";Needs to be ran through Prettier.
@ -0,0 +2,4 @@import Button from '../components/Button/Button';export default function ComponentLibrary() {const [darkMode, setDarkMode] = useState(() => {This file needs to be ran through Prettier.
@ -1,7 +1,8 @@import { defineConfig } from "vite";import react from "@vitejs/plugin-react-swc";import { defineConfig } from 'vite';This file needs to be ran through Prettier.