Use default color in Loader component for CSS variable #12844
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Adds proper fallback color handling for the Loader component to prevent invisible state when CSS variable --tw-button-color is undefined.
- Modified
packages/twenty-ui/src/feedback/loader/components/Loader.tsxto usetheme.font.color.tertiaryas fallback color when CSS variable is missing - Added comprehensive Storybook stories in
packages/twenty-ui/src/feedback/loader/__stories__/Loader.stories.tsxto test color variations and default behavior - Improves component resilience by ensuring loader remains visible even without explicit color props
- Stories include test assertions to verify visibility and styling correctness
2 files reviewed, 3 comments
Edit PR Review Bot Settings | Greptile
| // @ts-expect-error: Custom CSS variable for demonstration purposes | ||
| <div style={{ '--tw-button-color': 'blue' }}> | ||
| <Story /> |
There was a problem hiding this comment.
style: Custom CSS variable name should match existing naming convention. Consider using a more descriptive name like '--tw-loader-color' instead of '--tw-button-color' since this is for a loader component.
| // @ts-expect-error: Custom CSS variable for demonstration purposes | |
| <div style={{ '--tw-button-color': 'blue' }}> | |
| <Story /> | |
| // @ts-expect-error: Custom CSS variable for demonstration purposes | |
| <div style={{ '--tw-loader-color': 'blue' }}> | |
| <Story /> |
| const loaders = await waitFor(() => { | ||
| const elements = canvasElement.querySelectorAll('#container > *'); | ||
|
|
||
| expect(elements).toHaveLength(4); | ||
|
|
||
| return elements; | ||
| }); |
There was a problem hiding this comment.
style: Test could be more robust by checking if elements are not only present but also visible using .toBeVisible() for consistency with other stories.
| background-color: ${({ color, theme }) => | ||
| color ? theme.tag.text[color] : `var(--tw-button-color)`}; | ||
| color | ||
| ? theme.tag.text[color] | ||
| : `var(--tw-button-color, ${theme.font.color.tertiary})`}; |
There was a problem hiding this comment.
style: Duplicate logic from StyledLoaderContainer. Consider extracting to a shared const or function
Fixes a regression introduced by twentyhq#11639 cc @AMoreaux ## Before  ## After  https://github.com/user-attachments/assets/875c6c7e-3fc3-44bf-bb17-69ca362c1145
Fixes a regression introduced by #11639
cc @AMoreaux
Before
After
CleanShot.2025-06-24.at.17.23.21.mp4