Feature/custom themes #1409
Feature/custom themes #1409
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
This PR migrates the application's styling system from CSS-in-JS (vanilla-extract) to SCSS to support custom theme functionality.
- Added new
customStylesfield in settings-general.tsx but implementation is incomplete - value is stored but not used for theme application - Potential accessibility issues with hardcoded colors in new SCSS files (e.g. link.scss using #c0c1c7) instead of theme variables
- Backdrop blur effects in backdrop.scss may cause performance issues on lower-end devices
- Container query in profile-content.css.ts uses hardcoded 'app-container' string instead of imported variable
- Toast component positioning relies on hardcoded bottom panel height (26px) which could break if panel height changes
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
46 file(s) reviewed, 49 comment(s)
Edit PR Review Bot Settings | Greptile
| z-index: globals.$backdrop-z-index; | ||
| top: 0; | ||
| padding: calc(globals.$spacing-unit * 3); | ||
| backdrop-filter: blur(2px); |
There was a problem hiding this comment.
style: backdrop-filter has limited browser support and can cause performance issues. Consider adding -webkit-backdrop-filter for Safari support and a fallback for unsupported browsers.
| top: 0; | ||
| padding: calc(globals.$spacing-unit * 3); | ||
| backdrop-filter: blur(2px); | ||
| transition: all ease 0.2s; |
There was a problem hiding this comment.
style: transitioning 'all' properties can impact performance. Consider transitioning only specific properties like opacity and backdrop-filter.
| .backdrop { | ||
| animation-name: backdrop-fade-in; | ||
| animation-duration: 0.4s; | ||
| background-color: rgba(0, 0, 0, 0.7); |
There was a problem hiding this comment.
style: background-color values should be defined in globals.scss for theme customization support
|
|
||
| body { | ||
| overflow: hidden; | ||
| user-select: none; |
There was a problem hiding this comment.
style: user-select: none may cause accessibility issues by preventing text selection. Consider making this more selective or removing it.
| background: linear-gradient(270deg, #16b195 50%, #3e62c0 100%); | ||
| background-clip: text; | ||
| color: transparent; |
There was a problem hiding this comment.
syntax: background-clip: text requires -webkit prefix for full browser support
| background: linear-gradient(270deg, #16b195 50%, #3e62c0 100%); | |
| background-clip: text; | |
| color: transparent; | |
| -webkit-background-clip: text; | |
| background-clip: text; | |
| color: transparent; |
| &::-webkit-progress-bar { | ||
| background-color: globals.$dark-background-color; | ||
| } | ||
| &::-webkit-progress-value { | ||
| background-color: globals.$muted-color; | ||
| } |
There was a problem hiding this comment.
style: Progress bar styles only work in WebKit browsers. Consider adding standard progress element styles for cross-browser compatibility
| @@ -32,6 +32,8 @@ export function SettingsGeneral() { | |||
| repackUpdatesNotificationsEnabled: false, | |||
| achievementNotificationsEnabled: false, | |||
| language: "", | |||
|
|
|||
| customStyles: window.localStorage.getItem("customStyles") || "", | |||
There was a problem hiding this comment.
logic: customStyles is initialized from localStorage but never persisted back when changed via handleChange
| @@ -136,7 +138,7 @@ export function SettingsGeneral() { | |||
| }))} | |||
| /> | |||
|
|
|||
| <h3>{t("notifications")}</h3> | |||
| <p>{t("notifications")}</p> | |||
There was a problem hiding this comment.
style: changing from h3 to p reduces semantic meaning and affects page hierarchy
| "toast--closing": isClosing, | ||
| })} | ||
| > | ||
| <div className="toast__content"> | ||
| <div style={{ display: "flex", gap: `${SPACING_UNIT}px` }}> |
There was a problem hiding this comment.
style: inline styles should be moved to SCSS for consistency with the theming system
| $body-font-size: 14px; | ||
| $small-font-size: 12px; | ||
|
|
||
| $app-container: app-container; |
There was a problem hiding this comment.
style: $app-container should be prefixed with a namespace like $container-name-app to avoid potential naming collisions with other libraries
…her/hydra into feature/custom-themes
|
this pr add the possibility of create, import, edit and manage custom themes