Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Uses CSS module instead of Stitches for Loader component #2178

Merged
merged 2 commits into from
Jul 15, 2024
Merged

Uses CSS module instead of Stitches for Loader component #2178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14b60a2
Uses CSS module instead of Stitches for Loader component
infomiho Jul 12, 2024
5fb10c9
Newline
infomiho Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions waspc/data/Generator/templates/react-app/src/components/Loader.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
.loader {
color: var(--loader-color, #1a202c);
Copy link
Contributor Author

@infomiho infomiho Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrote it like this to enable users to adjust the loader color with CSS vars. It's undocumented but we can tell users that info if they ask.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is still local CSS, is it? Not global, right? Because of the ".module.css"? WHo is enabling this, Vite, or are we using something special?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vite supports CSS modules out of the box: https://vitejs.dev/config/shared-options.html#css-modules

}

.loaderCircle {
display: inline-block;
border-top: 2px solid currentcolor;
border-right: 2px solid currentcolor;
border-bottom: 2px solid transparent;
border-left: 2px solid transparent;
border-radius: 99999px;
animation: fullRotation 0.45s linear infinite;
width: var(--spinner-size);
height: var(--spinner-size);
--spinner-size: var(--loader-spinner-size, 2rem);
}

.loaderAccessibilityText {
border: 0px;
clip: rect(0px, 0px, 0px, 0px);
width: 1px;
height: 1px;
margin: -1px;
padding: 0px;
overflow: hidden;
white-space: nowrap;
position: absolute;
}

@keyframes fullRotation {
0% {
transform: rotate(0deg);
}
100% {
transform: rotate(360deg);
}
}
53 changes: 7 additions & 46 deletions waspc/data/Generator/templates/react-app/src/components/Loader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,12 @@
import { styled, keyframes } from 'wasp/core/stitches.config'
import styles from './Loader.module.css'

const fullRotationKeyframes = keyframes({
'0%': { transform: 'rotate(0deg)' },
'100%': { transform: 'rotate(360deg)' },
})

// DefaultLoader is a React component that spans accross the whole screen and displays
// a spinner in the very middle.
// Spans a accross the whole screen and displays a spinner in the very middle.
export function Loader() {
const loaderClassName = ['loader', styles.loader].join(' ')
return (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe just inline this, but do as you like.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw why do you also need to set 'loader' class here? I would imagine only styles.loader is enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe just inline this, but do as you like.

I prefer this 🤷

I would imagine only styles.loader is enough?

I wanted to give users a stable class name (styles.loader gives you a mangled name) if they wanted to style the loader in some way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, shouldn't we name it sometihng like wasp-loader? Because this is now global CSS class if I am right -> calling it just loader seems problematic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most people use Tailwind these days or one of the CSS-in-JS options, so I don't think there will be a lot of interference with built in classes. If it happens to happen, let's change it then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you are saying that our global CSS hopefully wont cause any issues because others might not use global CSS themselves? That is ok, but we can't count on that. Also, as a library/framework, we should at least try to not affect their global space if we can. I think we should do this now, it is an easy change and is a good practice and we know it has a potential to cause problems down the road: it would be downright sloppy to leave this as it is now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can add the wasp- prefix in a new PR.

<SpinnerWrapper className="loader">
<Spinner />
<SpinnerAccessbilityText>Loading...</SpinnerAccessbilityText>
</SpinnerWrapper>
<div className={loaderClassName}>
<div className={styles.loaderCircle} />
<div className={styles.loaderAccessibilityText}>Loading...</div>
</div>
)
}

const SpinnerWrapper = styled('div', {
color: '$gray900',
})

// Taken from Chakra UI Spinner component
const Spinner = styled('div', {
display: 'inline-block',
borderTop: '2px solid currentcolor',
borderRight: '2px solid currentcolor',
borderBottomStyle: 'solid',
borderLeftStyle: 'solid',
borderRadius: '99999px',
borderBottomWidth: '2px',
borderLeftWidth: '2px',
borderBottomColor: 'transparent',
borderLeftColor: 'transparent',
animation: `0.45s linear 0s infinite normal none running ${fullRotationKeyframes}`,
width: 'var(--spinner-size)',
height: 'var(--spinner-size)',
'--spinner-size': '2rem',
})

const SpinnerAccessbilityText = styled('span', {
border: '0px',
clip: 'rect(0px, 0px, 0px, 0px)',
width: '1px',
height: '1px',
margin: '-1px',
padding: '0px',
overflow: 'hidden',
whiteSpace: 'nowrap',
position: 'absolute',
})
1 change: 1 addition & 0 deletions waspc/e2e-test/test-outputs/waspBuild-golden/files.manifest
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion waspc/e2e-test/test-outputs/waspBuild-golden/waspBuild/.wasp/build/.waspchecksums
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions ...t-outputs/waspBuild-golden/waspBuild/.wasp/build/web-app/src/components/Loader.module.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 7 additions & 46 deletions ...est/test-outputs/waspBuild-golden/waspBuild/.wasp/build/web-app/src/components/Loader.tsx
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions waspc/e2e-test/test-outputs/waspCompile-golden/files.manifest
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion waspc/e2e-test/test-outputs/waspCompile-golden/waspCompile/.wasp/out/.waspchecksums
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions ...outputs/waspCompile-golden/waspCompile/.wasp/out/web-app/src/components/Loader.module.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

53 changes: 7 additions & 46 deletions ...t/test-outputs/waspCompile-golden/waspCompile/.wasp/out/web-app/src/components/Loader.tsx
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions waspc/e2e-test/test-outputs/waspComplexTest-golden/files.manifest
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion waspc/e2e-test/test-outputs/waspComplexTest-golden/waspComplexTest/.wasp/out/.waspchecksums
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading