-
Notifications
You must be signed in to change notification settings - Fork 0
Create centralized button component #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is only the first version. It still needs the rest of the styles, and plenty of other functionality.
Added another check to prop validation, and removed the dependency on the useCheckMobile hook so it can be used on server-rendered components.
This is to ensure that each style of button has a consistent size. I know it's ugly, but it wouldn't work otherwise.
It's just the usual combination of CSS rules that center an absolutely-positioned element, all bundled into one helpful class.
When the button is calling the onClick function, it puts the button in a state where it cannot be clicked and shows a loading spinner.
This helps with the case where a button would create a resource, then navigate to a different page. If the user clicks the button again before the page loads, they could unintentionally create a second resource.
Co-Authored-By: Miranda Zheng <[email protected]>
This makes it so you can specify the border color without it getting overridden by the default values. Co-Authored-By: Miranda Zheng <[email protected]>
Co-Authored-By: Miranda Zheng <[email protected]>
As this grows, it makes sense to keep it contained here. This is mostly because I want to separate out some of the types into another file.
The props and style types were also moved to an external file to handle this. Essentially, the props now have sets of possible parameters that can be specified, like isLink with href. This should help developers when using it, both making sure that they add all the right props and making sure they don't specify unneeded props. I just hope the error messages are helpful enough...
I'll get to it eventually, just not now.
I'll be honest, sometimes the comments don't show up in VS Code, but other times they do. Hopefully this structure of types doesn't cause problems later.
I know it seems more complicated than it needs to be, but for whatever reason the frosted glass button wouldn't show an outline unless group-focus was used.
This is to allow for external control of the loading state. This is probably only going to be used in the new account button later.
It makes much more sense to create two types of buttons for developers to use instead of trying to trust them to use the right props.
These should be called instead of the BaseButton class.
This was all caused by the Radix UI Dropdown menu. It needed a forwardRef to activate the dropdown component. In addition, it turns out that the "style" component was conflicting with an existing HTML component, so the name is now changed.
The buttons now have icons, to hopefully reinforce what those icons mean.
The larger icons were getting a bit too close to the rounded button border.
Arrays of buttons will be reused multiple times in the codebase. Why not make it a central type?
This houses buttons in a frosted glass container at the bottom of the screen. All it needs is a list of buttons to display.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces a comprehensive button component system to standardize button usage across the application, refactors form submission handlers to return success/failure booleans, and enhances styling consistency throughout the codebase.
- Introduces new reusable
ActionButtonandLinkButtoncomponents with unified styling and loading states - Refactors async form submission handlers to return boolean values indicating success/failure
- Creates a mobile footer tray component for consistent button layouts on mobile devices
Reviewed Changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/features/button/props.ts | Defines TypeScript types for button component props with detailed documentation |
| src/features/button/components/base-button.tsx | Implements core button component with loading states, styling variants, and forwardRef support |
| src/features/button/components/action-button.tsx | Wrapper component for action buttons that execute onClick handlers |
| src/features/button/components/link-button.tsx | Wrapper component for navigation buttons using Next.js Link |
| src/features/button/button-array.ts | Type definition for arrays of button components |
| src/components/mobile-footer-tray.tsx | Reusable mobile footer component that displays button arrays |
| src/components/loading-spinner.tsx | Animated spinner component used in loading button states |
| src/lib/utils/api/submit-event.ts | Refactored to return boolean and use try-catch instead of promise chains |
| src/features/event/editor/editor.tsx | Updated to use new button components and handle boolean return values |
| src/app/(event)/[event-code]/painting/page-client.tsx | Updated to use new button components with cancel and submit actions |
| src/app/(event)/[event-code]/page-client.tsx | Replaced custom button implementations with LinkButton components |
| src/components/header/*.tsx | Converted all header buttons to use new button component system |
| src/app/(auth)/*.tsx | Refactored authentication forms to use ActionButton/LinkButton and return booleans |
| src/app/page.tsx | Updated landing page CTAs to use LinkButton components |
| src/app/error.tsx | Updated error page button to use ActionButton |
| src/styles/centered-absolute.css | New utility class for centered absolute positioning |
| src/styles/frosted-glass.css | Enhanced with button-specific hover and loading states |
| src/features/dashboard/components/*.tsx | Updated to use consistent foreground colors and simplified styling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This fixes an issue on Chrome where the focus outline would show on click.
mirmirmirr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This PR introduces the brand-new centralized button component. This creates a more modular system for interactions, and the existing buttons on the site are now more reactive and provide more feedback to the user. They are also more easily styled from a central component.
As always, a few extra fixes are here and there.
New Button Component
This new component comes in two flavors:
LinkButton- Used for navigation, turns into aLinkcomponent from Next.js.ActionButton- Used for submissions and running functions.Each of these buttons can be in one of 4 styles:
primary,secondary,frosted glass, andtransparent. These should be used depending on the hierarchy of actions on the page.This button also implements
forwardRef, so it's compatible with Radix UI components as a trigger.Replacement of Existing Buttons
Most of the existing buttons on the page were replaced with this new button component for maintainability and a consistent style. The ones that weren't replaced are unique and have a different style from the new button component.
New
MobileFooterTrayComponentFor the event editor and painting pages, a new footer tray was created to house the new button component.
The existing button in the footer had a unique styling, but it needed to have the same functionality that the new button component offered. This new tray solves this problem. It can also support multiple buttons in the tray at once (e.g. Submit and Cancel buttons).
New
centered-absoluteCSS ClassJust a utility class that, when applied to an element, positions it absolutely and centered within the nearest relative parent.
This was used to get the loading spinner positioned correctly on the button component.