feat: Adicionei a nova feature de dark-mode#669
Conversation
📝 WalkthroughWalkthroughThis PR adds a theme system enabling users to select between Light and Dark modes. It introduces a ChangesTheme System Implementation
Sequence DiagramsequenceDiagram
actor User
participant UI as DropdownIcon Component
participant Service as ThemeService
participant Storage as localStorage
participant DOM as Document Body
User->>UI: Click "Light" or "Dark" button
UI->>Service: setTheme('light'/'dark')
Service->>Storage: Write to 'brmw-theme' key
Service->>DOM: Toggle theme-light/theme-dark class
DOM->>DOM: Apply dark theme CSS variables & styles
Service-->>UI: Return updated theme
UI->>UI: Update active button state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/angular/service/themeService.js (1)
28-31:toggleTheme()lacks state sync with dropdown controller
toggleTheme()is defined in the service but not currently called elsewhere in the codebase. However, the method exposes a design vulnerability: if invoked from outside the component (e.g., a future keyboard shortcut handler), it will apply the new theme to the DOM without updatingctrl.themeindropdownIcon.js, causing the dropdown highlight to display stale state. While this is not a live bug today, the exported method should either be removed or updated to syncctrl.themewhen the theme changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/angular/service/themeService.js` around lines 28 - 31, toggleTheme currently applies the new theme to the DOM but doesn't update dropdownIcon's ctrl.theme, risking stale UI; either remove toggleTheme from the service or change it to synchronise state by notifying listeners after applying the theme. Fix by modifying service.toggleTheme (using service.getTheme and service.applyTheme) to emit a theme change notification (e.g. $rootScope.$broadcast('theme:changed', nextTheme) or call a registered callback) immediately after applyTheme resolves, and update dropdownIcon.js to listen for that 'theme:changed' event and set ctrl.theme accordingly; alternatively make toggleTheme accept an optional callback to update ctrl.theme and call it with nextTheme.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/angular/components/dropdownIcon.html`:
- Around line 9-22: The two theme toggle buttons (the elements using
$ctrl.setTheme(...) and ng-class that checks $ctrl.theme) lack an ARIA pressed
state; update both button elements to emit aria-pressed reflecting whether
$ctrl.theme equals 'light' or 'dark' (use ng-attr-aria-pressed with an
expression that produces "true"/"false") so screen readers can announce the
active toggle while leaving existing ng-class and $ctrl.setTheme intact.
In `@app/angular/service/themeService.js`:
- Around line 13-15: getTheme() currently returns any string stored under
storageKey which can initialize ctrl.theme to invalid values; update
service.getTheme to validate the retrieved value against the allowed themes
(themes.light and themes.dark) and return a safe default (e.g., themes.light)
when the stored value is not one of those two, ensuring components like
dropdownIcon.js (ctrl.theme) only receive "light" or "dark".
---
Nitpick comments:
In `@app/angular/service/themeService.js`:
- Around line 28-31: toggleTheme currently applies the new theme to the DOM but
doesn't update dropdownIcon's ctrl.theme, risking stale UI; either remove
toggleTheme from the service or change it to synchronise state by notifying
listeners after applying the theme. Fix by modifying service.toggleTheme (using
service.getTheme and service.applyTheme) to emit a theme change notification
(e.g. $rootScope.$broadcast('theme:changed', nextTheme) or call a registered
callback) immediately after applyTheme resolves, and update dropdownIcon.js to
listen for that 'theme:changed' event and set ctrl.theme accordingly;
alternatively make toggleTheme accept an optional callback to update ctrl.theme
and call it with nextTheme.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27b0b660-5f2a-4757-994b-15aac7711a0e
📒 Files selected for processing (10)
app/angular/components/dropdownIcon.htmlapp/angular/components/dropdownIcon.jsapp/angular/index.jsapp/angular/service/themeService.jsapp/i18n/languages/en.jsapp/i18n/languages/pt_BR.jsapp/sass/colors.scssapp/sass/joint-custom.scssapp/sass/mainHeader.scssapp/sass/structure.scss
| <button | ||
| type="button" | ||
| ng-class="{active: $ctrl.theme === 'light'}" | ||
| ng-click="$ctrl.setTheme('light')" | ||
| > | ||
| {{ 'Light' | translate }} | ||
| </button> | ||
| <button | ||
| type="button" | ||
| ng-class="{active: $ctrl.theme === 'dark'}" | ||
| ng-click="$ctrl.setTheme('dark')" | ||
| > | ||
| {{ 'Dark' | translate }} | ||
| </button> |
There was a problem hiding this comment.
Add aria-pressed so screen readers can announce the active theme.
Per the WAI-ARIA Authoring Practices Guide, if the button is a toggle button, it must carry an aria-pressed state. Currently only the visual .active class communicates which theme is selected; state changes fail completely when you rely on visual-only indicators like colour changes.
♿ Proposed fix — add aria-pressed to both buttons
<button
type="button"
ng-class="{active: $ctrl.theme === 'light'}"
+ ng-attr-aria-pressed="{{ $ctrl.theme === 'light' }}"
ng-click="$ctrl.setTheme('light')"
>
{{ 'Light' | translate }}
</button>
<button
type="button"
ng-class="{active: $ctrl.theme === 'dark'}"
+ ng-attr-aria-pressed="{{ $ctrl.theme === 'dark' }}"
ng-click="$ctrl.setTheme('dark')"
>
{{ 'Dark' | translate }}
</button>ng-attr-aria-pressed is used instead of aria-pressed directly to emit the attribute with the correct boolean string ("true" / "false"), which AngularJS expression interpolation alone would leave as the JS boolean value.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| ng-class="{active: $ctrl.theme === 'light'}" | |
| ng-click="$ctrl.setTheme('light')" | |
| > | |
| {{ 'Light' | translate }} | |
| </button> | |
| <button | |
| type="button" | |
| ng-class="{active: $ctrl.theme === 'dark'}" | |
| ng-click="$ctrl.setTheme('dark')" | |
| > | |
| {{ 'Dark' | translate }} | |
| </button> | |
| <button | |
| type="button" | |
| ng-class="{active: $ctrl.theme === 'light'}" | |
| ng-attr-aria-pressed="{{ $ctrl.theme === 'light' }}" | |
| ng-click="$ctrl.setTheme('light')" | |
| > | |
| {{ 'Light' | translate }} | |
| </button> | |
| <button | |
| type="button" | |
| ng-class="{active: $ctrl.theme === 'dark'}" | |
| ng-attr-aria-pressed="{{ $ctrl.theme === 'dark' }}" | |
| ng-click="$ctrl.setTheme('dark')" | |
| > | |
| {{ 'Dark' | translate }} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/angular/components/dropdownIcon.html` around lines 9 - 22, The two theme
toggle buttons (the elements using $ctrl.setTheme(...) and ng-class that checks
$ctrl.theme) lack an ARIA pressed state; update both button elements to emit
aria-pressed reflecting whether $ctrl.theme equals 'light' or 'dark' (use
ng-attr-aria-pressed with an expression that produces "true"/"false") so screen
readers can announce the active toggle while leaving existing ng-class and
$ctrl.setTheme intact.
| service.getTheme = () => { | ||
| return $window.localStorage.getItem(storageKey) || themes.light; | ||
| }; |
There was a problem hiding this comment.
getTheme() returns an unvalidated localStorage value
If localStorage["brmw-theme"] holds anything other than "light" or "dark" (e.g., manual tampering or a future migration), ctrl.theme is initialized to that arbitrary string in dropdownIcon.js line 8. The dropdown ng-class comparison will silently match nothing, leaving both options un-highlighted even though applyTheme at startup normalizes and correctly sets the body class.
🛡️ Proposed fix — validate against known themes
service.getTheme = () => {
- return $window.localStorage.getItem(storageKey) || themes.light;
+ const stored = $window.localStorage.getItem(storageKey);
+ return Object.values(themes).includes(stored) ? stored : themes.light;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| service.getTheme = () => { | |
| return $window.localStorage.getItem(storageKey) || themes.light; | |
| }; | |
| service.getTheme = () => { | |
| const stored = $window.localStorage.getItem(storageKey); | |
| return Object.values(themes).includes(stored) ? stored : themes.light; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/angular/service/themeService.js` around lines 13 - 15, getTheme()
currently returns any string stored under storageKey which can initialize
ctrl.theme to invalid values; update service.getTheme to validate the retrieved
value against the allowed themes (themes.light and themes.dark) and return a
safe default (e.g., themes.light) when the stored value is not one of those two,
ensuring components like dropdownIcon.js (ctrl.theme) only receive "light" or
"dark".
1 participant
Dark Mode
Resumo
Este PR adiciona suporte a dark mode na aplicação e um seletor de tema no menu de perfil do usuário.
O que foi feito
ThemeServicepara salvar o tema selecionado nolocalStorage;Arquivos alterados
app/angular/service/themeService.jsapp/angular/index.jsapp/angular/components/dropdownIcon.jsapp/angular/components/dropdownIcon.htmlapp/i18n/languages/pt_BR.jsapp/i18n/languages/en.jsapp/sass/colors.scssapp/sass/mainHeader.scssapp/sass/structure.scssapp/sass/joint-custom.scssValidação
npx webpack --progress --color;Observações
brmw-theme;theme-darknobody, sem alterar o comportamento atual do tema claro.Summary by CodeRabbit