-
Notifications
You must be signed in to change notification settings - Fork 1
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
Created custom button variants #132
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Everything looks good to me! Although you should look into the PR failing the Vercel check.
Buh, it was still running when I last checked and didn't see it failed. I'll have to look into why. |
@ryandotfurrer |
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.
@ryandotfurrer please review my comments.
@ryandotfurrer please adjust the Storybook story to reflect and show these variants as well. This is the way @jasinn will review design implementation. Also, add unit test for these components to ensure the variants are showing the correct styles. We should look into adding visual tests with Storybook 8 as well. |
I will make these updates (and your above-mentioned ones) by EoD 4/16/24. |
package.json
Outdated
@@ -47,15 +47,13 @@ | |||
"@types/node": "20.3.1", | |||
"@types/react": "18.2.8", | |||
"@types/react-dom": "18.2.5", | |||
"@typescript-eslint/eslint-plugin": "^7.6.0", |
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.
Why are these packages being removed? I'm pretty sure this was not intentional.
stories/About/Developer Guide.mdx
Outdated
@@ -32,3 +32,24 @@ import client from '../assets/client_component.png'; | |||
<code>git push --set-upstream origin richard/user-picks-page</code> | |||
|
|||
Subsequent pushes will now go to your remote upstream branch. | |||
|
|||
## Connecting to <del>Supabase</del> |
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.
I think we need to talk about merge conflicts. :D This is not the first time Supabase came back into your PR.
Remove unnecessary space in Button log out component. Co-authored-by: Shashi Lo <[email protected]>
Closes #155 # Created nav component ![Screenshot of Nav component in dark mode](https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/954db134-5f79-47d3-aa6f-df152bf9b4b8) https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/0e143fbb-a8f7-45b9-9465-0983c3dfbc10 ## Todo - [x] Add hamburger icon ~~- [ ] Add logic to show sign out only if user is signed in (?)~~ Not needed for MVP - [x] Integrate `Button.tsx` component from #132 and use in place of standard `<button>` element --------- Co-authored-by: Shashi Lo <[email protected]>
Closes #155 # Created nav component ![Screenshot of Nav component in dark mode](https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/954db134-5f79-47d3-aa6f-df152bf9b4b8) https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/0e143fbb-a8f7-45b9-9465-0983c3dfbc10 ## Todo - [x] Add hamburger icon ~~- [ ] Add logic to show sign out only if user is signed in (?)~~ Not needed for MVP - [x] Integrate `Button.tsx` component from #132 and use in place of standard `<button>` element --------- Co-authored-by: Shashi Lo <[email protected]>
Closes issue #124 ## Change Description(s) -Edited Button component to customize to our colors - Commented out the initial "default" and "outline" variants and made my own. - [Our design](https://www.figma.com/file/jJXt7OmmwSvHLpkFa8zEDh/Gridiron-Survivor-App?type=design&node-id=1-153&mode=design&t=yuGQC9u2T3Dxgmvn-0) currently only uses one type of button, but I thought it wise to at least add one other variant. - ![Screenshot of Gridiron Survivor login screen](https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/d1b16b72-884a-40cf-a8f3-ca775443b9de) - I also edited the base button component, not just our variants, to be the full-width of their container as no part of our design has a button that is not as wide as the parent container. ## Finished Results Below is a screenshot of their buttons. I placed them inside a div with a border and some padding so you could see that they are the full-width of the parent container (minus the padding). ![Screenshot of current Gridiron Survivor homepage showcasing the two button variants](https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/334f1ec5-c184-433c-ae14-71fb7277576d) --------- Co-authored-by: Shashi Lo <[email protected]>
Closes #155 # Created nav component ![Screenshot of Nav component in dark mode](https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/954db134-5f79-47d3-aa6f-df152bf9b4b8) https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/0e143fbb-a8f7-45b9-9465-0983c3dfbc10 ## Todo - [x] Add hamburger icon ~~- [ ] Add logic to show sign out only if user is signed in (?)~~ Not needed for MVP - [x] Integrate `Button.tsx` component from #132 and use in place of standard `<button>` element --------- Co-authored-by: Shashi Lo <[email protected]>
Closes #155 ![Screenshot of Nav component in dark mode](https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/954db134-5f79-47d3-aa6f-df152bf9b4b8) https://github.com/LetsGetTechnical/gridiron-survivor/assets/40150036/0e143fbb-a8f7-45b9-9465-0983c3dfbc10 - [x] Add hamburger icon ~~- [ ] Add logic to show sign out only if user is signed in (?)~~ Not needed for MVP - [x] Integrate `Button.tsx` component from #132 and use in place of standard `<button>` element --------- Co-authored-by: Shashi Lo <[email protected]>
Closes issue #124
Change Description(s)
-Edited Button component to customize to our colors
Finished Results
Below is a screenshot of their buttons. I placed them inside a div with a border and some padding so you could see that they are the full-width of the parent container (minus the padding).