feat: migration to scss #1441
feat: migration to scss #1441
Conversation
…hydralauncher/hydra into feat/migration-to-scss-3-remake
# Conflicts: # src/renderer/src/app.css.ts # src/renderer/src/components/toast/toast.tsx # src/renderer/src/pages/game-details/modals/repacks-modal.scss # src/renderer/src/pages/game-details/modals/repacks-modal.tsx # src/renderer/src/pages/game-details/sidebar/how-long-to-beat-section.tsx # src/renderer/src/pages/home/home.tsx # src/renderer/src/theme.css.ts
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements a comprehensive migration from Vanilla Extract CSS-in-TS to SCSS across the entire application. Here's a concise summary of the key changes:
- Removed all Vanilla Extract dependencies (@vanilla-extract/css, @vanilla-extract/dynamic, @vanilla-extract/recipes) and added sass-embedded for SCSS support
- Migrated all .css.ts files to .scss, implementing BEM methodology for class naming and proper SCSS nesting
- Replaced theme.css.ts variables with SCSS global variables in globals.scss
- Updated electron.vite.config.ts with SCSS preprocessor configuration
- Converted inline styles to SCSS classes across components
Key points to review:
- Verify that hardcoded color values (e.g. '#1c1c1c' in profile.tsx) are moved to SCSS variables for consistency
- Check that fixed widths (e.g. 500px in add-download-source-modal.scss) are responsive
- Ensure all animations and transitions are properly migrated from keyframes
- Review accessibility of color contrast with new SCSS implementation
- Test responsive layouts thoroughly as spacing units changed from dynamic to static values
172 file(s) reviewed, 144 comment(s)
Edit PR Review Bot Settings | Greptile
| backdrop-filter: blur(2px); | ||
| transition: all ease 0.2s; |
There was a problem hiding this comment.
style: transition: all is expensive for performance. Consider transitioning only backdrop-filter and background-color properties.
| @@ -7,5 +7,6 @@ | |||
| border: solid 1px globals.$muted-color; | |||
| border-radius: 4px; | |||
| display: flex; | |||
| gap: 4px; | |||
There was a problem hiding this comment.
style: consider using globals.$spacing-unit / 2 to maintain consistency with the padding definition on line 6
| &:hover:not(:has(input:disabled)) { | ||
| border-color: rgba(255, 255, 255, 0.5); | ||
| } |
There was a problem hiding this comment.
style: The hover state uses a hardcoded rgba value instead of a global variable. Consider using a theme variable for consistency.
| justify-content: center; | ||
| align-items: center; | ||
| position: relative; | ||
| transition: all ease 0.2s; |
There was a problem hiding this comment.
style: transition: all is performance-intensive. Consider specifying only the properties that need to transition (border-color, opacity).
| <div className={styles.checkbox({ checked: props.checked })}> | ||
| <div className="checkbox-field"> | ||
| <div | ||
| className={`checkbox-field__checkbox ${props.checked ? "checked" : ""}`} |
There was a problem hiding this comment.
style: template literal could be simplified to use BEM modifier class instead
| flex-direction: column; | ||
| gap: calc(globals.$spacing-unit * 2); | ||
| max-height: 400px; | ||
| overflow-y: scroll; |
There was a problem hiding this comment.
style: Use overflow-y: auto instead of scroll to prevent showing scrollbar when not needed
| overflow-y: scroll; | |
| overflow-y: auto; |
| &__submit-button { | ||
| align-self: flex-end; | ||
| width: 100%; |
There was a problem hiding this comment.
logic: The submit button has both align-self: flex-end and width: 100%, which appears contradictory since a full-width element cannot be aligned to the end
| } | ||
|
|
||
| &__button { | ||
| align-self: end; |
There was a problem hiding this comment.
style: align-self: end is not as widely supported as align-self: flex-end
| align-self: end; | |
| align-self: flex-end; |
| display: flex; | ||
| gap: calc(globals.$spacing-unit / 2); | ||
| align-items: center; | ||
| transition: all ease 0.2s; |
There was a problem hiding this comment.
style: transition: all ease 0.2s is performance-intensive. Consider transitioning only the color property since that's all that changes on hover.
| transition: all ease 0.2s; | |
| transition: color ease 0.2s; |
| &__friend-code-button { | ||
| color: globals.$body-color; | ||
| cursor: pointer; | ||
| display: flex; | ||
| gap: calc(globals.$spacing-unit / 2); | ||
| align-items: center; | ||
| transition: all ease 0.2s; | ||
|
|
||
| &:hover { | ||
| color: globals.$muted-color; | ||
| } | ||
| } |
There was a problem hiding this comment.
logic: friend-code-button lacks focus styles for keyboard navigation. Should match hover state for accessibility.
| &__friend-code-button { | |
| color: globals.$body-color; | |
| cursor: pointer; | |
| display: flex; | |
| gap: calc(globals.$spacing-unit / 2); | |
| align-items: center; | |
| transition: all ease 0.2s; | |
| &:hover { | |
| color: globals.$muted-color; | |
| } | |
| } | |
| &__friend-code-button { | |
| color: globals.$body-color; | |
| cursor: pointer; | |
| display: flex; | |
| gap: calc(globals.$spacing-unit / 2); | |
| align-items: center; | |
| transition: color ease 0.2s; | |
| &:hover, | |
| &:focus { | |
| color: globals.$muted-color; | |
| } | |
| } |
…hydralauncher/hydra into feat/migration-to-scss-3-remake
…hydralauncher/hydra into feat/migration-to-scss-3-remake
|
No description provided.