-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add ProgressBar and BackButton Components #129
Conversation
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.
Looks great aside from a few small things! I really appreciate you getting ahead on this so early :D Let me know if you have any questions about the comments or how to do anything if you're new to any of the tools!
web/components/atoms/ProgressBar.tsx
Outdated
}: Props) { | ||
return ( | ||
<div className="h-[6px] relative rounded-full"> | ||
<div style={{ backgroundColor: color, width: `${progress}%` }} className="rounded-full absolute top-0 left-0 bottom-0 transition-all" /> |
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 like how you made color a variable in case we want to have different colors in the future, but I think it'll only be MBB's pink, so you can statically set this color as "bg-mbb-pink"
in Tailwind since we define "mbb-pink" in web/tailwind.config.js
!
web/components/atoms/ProgressBar.tsx
Outdated
progress | ||
}: Props) { | ||
return ( | ||
<div className="h-[6px] relative rounded-full"> |
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.
Can you also add the gray on the other side of the progress bar? I think you can just add bg-[color]
to this div, and you can add the gray color to web/tailwind.config.js
too so that it can be easily reused and updated!
Can I add multiple components in this PR? I also finished the back button |
web/components/atoms/BackButton.tsx
Outdated
|
||
export default function BackButton({ onClick, disabled }: Props) { | ||
return ( | ||
<button className="group flex items-center gap-[2px] text-[#8C8C8C]" onClick={onClick} disabled={disabled}> |
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.
The back button looks great! Thanks for adding the hover color too, I appreciate the attention to detail. Quick code change though, instead of using text-[#8C8C8C]
, you can use text-medium-gray
from Tailwind config, and instead of stroke-gray-500
use stroke-dark-gray
(also in Tailwind config) so we can use consistent gray colors throughout the codebase! I'll figure out how to name these colors on Figma so that they'll show up in Dev Mode in the future
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.
Alright, will do
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.
We just need two very small changes and then we'll be good to merge it in!
web/components/atoms/BackButton.tsx
Outdated
> | ||
<path | ||
d='M15 18L9 12L15 6' | ||
className={`${disabled ? 'stroke-dark-gray' : 'group-hover:stroke-dark-gray'}`} |
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.
Add stroke-medium-gray
in className
so that the chevron appears when the button isn't disabled 😅
web/components/atoms/BackButton.tsx
Outdated
/> | ||
</svg> | ||
<div | ||
className={`${disabled ? 'text-stroke-dark-gray' : 'group-hover:text-stroke-dark-gray'}`} |
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.
Since we're just coloring the text in the div, these should both be text-dark-gray
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.
Components look great 🎉 Thanks for getting these in early!
What does this Pull Request change?
Added a
ProgressBar
andBackButton
component toatoms
as part of #122Important Changes