-
Notifications
You must be signed in to change notification settings - Fork 0
Finalize FAB: fixed placement and tokens-based styling #2
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
base: main
Are you sure you want to change the base?
Conversation
JasminePRA
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.
added comments
| | 'secondary' | ||
| | 'danger' | ||
| | 'success' | ||
| | 'warning' |
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 remove info?
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 info dont have a color in theme, I add a color for info in theme and it will work.
| | 'success' | ||
| | 'warning' | ||
| export type FabTextColor = 'primary' | 'secondary' | 'white' | ||
| export type Fabsize = 'SMALL' | 'MEDIUM' | 'LARGE' |
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 capitalized?
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 the size is linked to the token and token is written in Uppercase. I change the Fabsize in token to sm, md, lg. And update the Fab component
| | 'danger' | ||
| | 'success' | ||
| | 'warning' | ||
| export type FabTextColor = 'primary' | 'secondary' | 'white' |
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 white? primary and secondary should be fine
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.
It depends on your current primary and secondary text colors. I can update the secondary color to white, and then it will work.
| bgColor?: FabBgColor | ||
| textColor?: FabTextColor | ||
| size?: Fabsize | ||
| position?: 'absolute' | 'fixed' | 'relative' | ||
| bottom?: number | ||
| right?: number | ||
| top?: number | ||
| left?: number | ||
| zIndex?: number |
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 they all optional? think about what is necessary for the fab to be a fab
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.
Updated.
| left: ${({ left }) => left ?? 'auto'}; | ||
| z-index: ${({ zIndex }) => zIndex ?? 1000}; | ||
| `; | ||
| export const Container = styled.button<{ |
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.
a button is not a container though
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.
Change to div
…condary textColor to white.
…use. Change to Fab element to DIV.
Pull Request
Description
This PR finalizes the
Fab(Floating Action Button) component.It includes:
+icon)react-iconsfor default icon supportonClickhandlingHow to test
Install and start the project:
npm installnpm run devAdd the Fab component to App.tsx for testing
import Fab from './components/common/atoms/Fab/Fab' function App() { const handleClick = () => { alert('Clicked!') } return ( <> < Fab onClick={handleClick} />bgColor, orsize< Fab onClick={handleClick} bgColor="secondary" size="LARGE" />Type of Change
feature: New featurechore: Maintenance, dependency updates, or refactoringtest: Adding or improving testsbug: Bug fixdocs: Documentation updatesShortcut story
https://trello.com/c/vVWL3MCh
Related Issues
Testing Performed
Screenshots/Recordings
Checklist
Additional Notes