release/15_10_2025 #444
release/15_10_2025 #444
Conversation
WalkthroughAdds a data-theme driven theming system with a runtime toggle, wires theme through shell/header and consumers, introduces keyboard focus management for category pills and a FocusableItemDirective library, exposes hideGradient/articleCard/imagePriority inputs, and updates many templates, styles, and Tailwind tokens for light/dark variants. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Header as HeaderComponent
participant Shell as RootShellComponent
participant Store as AppThemeStore
participant DOM as Document(:root)
participant Consumer as Component (AboutUs / Giscus / Cards)
User->>Header: click theme toggle
Header-->>Shell: emit themeToggle
Shell->>Store: toggleTheme()
Store->>DOM: set data-theme attribute (dark|light)
Store->>Store: patchState(theme)
Store-->>Consumer: theme() signal updates
note right of Consumer: Consumers read theme() and adjust hideGradient, classes, giscus theme, image priorities
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
libs/blog/become-author/feature-become-author-page/src/lib/become-author-page/become-author-page.component.html (2)
34-34: Consider using theme-aware text color for improved theme compatibilityWhile white text works well on the pink background, if the button background color might change based on theme, consider using a theme-aware text color class.
If the button background remains pink in all themes, this change is fine. Otherwise, consider:
- <p class="text-sm font-bold text-white"> + <p class="text-sm font-bold text-al-pink-foreground">This assumes you have defined appropriate foreground colors for your themed backgrounds.
64-64: Consider using theme-aware text color for improved theme compatibilitySimilar to the other button, if the background color remains pink in all themes, white text is appropriate. For full theme support, consider a variable-based approach.
If the button background remains pink in all themes, this change is fine. Otherwise, consider using a theme-aware text color that complements the button background.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/blog/src/assets/icons/arrow-down.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html(2 hunks)libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts(0 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts(2 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card-skeleton.component.ts(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html(3 hunks)libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.html(1 hunks)libs/blog/articles/ui-article-list-title/src/lib/ui-article-list-title/ui-article-list-title.component.html(1 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html(1 hunks)libs/blog/become-author/feature-become-author-page/src/lib/become-author-page/become-author-page.component.html(2 hunks)libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-logo.component.ts(2 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.scss(1 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html(3 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts(1 hunks)libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html(1 hunks)libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html(1 hunks)libs/blog/shared/ui-card/src/lib/card.component.ts(2 hunks)libs/blog/shared/ui-card/src/lib/card.stories.ts(1 hunks)libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts(3 hunks)libs/blog/shared/ui-pill/src/lib/pill.directive.ts(1 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html(1 hunks)libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html(2 hunks)libs/shared/assets/src/lib/styles/main.scss(1 hunks)package.json(1 hunks)tailwind.preset.js(2 hunks)
💤 Files with no reviewable changes (1)
- libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (1)
libs/blog/shared/types/src/lib/article-card.ts (1)
ArticleCard(1-13)
🔇 Additional comments (48)
package.json (1)
67-67: Verify plugin version compatibility
The plugin is added as a beta release; please confirm that2.0.0-beta.0oftailwindcss-theme-variantsis compatible with Tailwind CSS^3.3.6and appropriate for production use.libs/blog/shared/ui-card/src/lib/card.stories.ts (1)
61-61: Demonstration of transparent background variant
This story correctly showcases the transparent background use case by applyingbg-transparentto the<al-card>element.libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html (1)
8-8: Consistent use of primary-foreground token
Applyingtext-al-primary-foregroundensures that the heading color adapts to theme variants and remains consistent across light/dark modes.libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card-skeleton.component.ts (1)
10-10: Consistent transparent background
Addingbg-transparentaligns the skeleton card with the new light-mode theming, ensuring the skeleton blends correctly across themes.libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html (1)
3-3: Verify removal of fixed height
Removingmax-h-52in favor of theme-based styling could affect the card's vertical sizing. Please verify that the card height remains consistent and does not overflow its container across breakpoints.libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html (1)
45-45: Apply theme-aware text color
Addingtext-al-foregroundensures the author name responds to light/dark mode variants and aligns with the new theming tokens. Looks good.libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html (1)
6-6: Verify dark mode background color
Withbg-whitestill applied, this element may not adapt its background in dark mode. Please confirm thatbg-whiteis intended, or consider using a theme-variant class (e.g.,dark:bg-al-card) so the background aligns with dark theme.libs/blog/articles/ui-article-list-title/src/lib/ui-article-list-title/ui-article-list-title.component.html (1)
4-4: Standardize primary heading color
Addingtext-al-primary-foregroundensures the title adheres to the new primary text color token and is consistent with other headings across the app.libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.html (1)
15-18: LGTM! Improved consistency across card variantsAdding the
imagePriorityinput to the compact card variant brings consistency with the other card types and enables image loading optimization.libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html (2)
14-14: Good addition of explicit text colorAdding the
text-whiteclass ensures the step numbers maintain proper contrast with the pink background in both light and dark modes.
49-49: Proper contrast for button textThe addition of
text-whiteensures the button text has adequate contrast against the pink background regardless of theme, supporting accessibility.libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html (3)
2-2: Good use of semantic color tokenUsing
text-al-primary-foregroundfor the title enables proper theme adaptation instead of a hard-coded color value.
6-6: Appropriate background setting for theme supportSetting the card to
bg-transparentallows it to properly inherit background colors when switching between light and dark themes.
38-38: Consistent use of semantic color tokenUsing
text-al-primary-foregroundfor the authors title maintains styling consistency with the main title and supports theme adaptation.libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html (3)
9-9: Improved layout with centered navigationAdding
justify-centerimproves the horizontal navigation layout by properly centering the navigation items.
24-25: Good implementation of theme-aware text stylingReplacing static text color with conditional styling
[ngClass]="whiteFont() ? 'text-white' : 'text-al-foreground'"enables proper text color adaptation based on context and theme.Make sure the
whiteFont()input property is properly implemented in the component TypeScript file and documented for consumers of this component.
37-38: Consistent styling for external linksThe same conditional text styling is correctly applied to external links, maintaining styling consistency across all navigation items.
libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-logo.component.ts (1)
2-2: Good component simplificationRemoving the unused
inputimport and thecurrentYearproperty simplifies the component appropriately. This cleanup removes unnecessary functionality while maintaining the component's core purpose.Ensure that any components using
<al-footer-logo>no longer pass thecurrentYearinput property, as it has been removed.Also applies to: 23-23
libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html (3)
3-5: Consistent trailing comma in[ngClass]object literal
The trailing comma after the single entry is valid and aligns with our formatting conventions for multiline object literals.
11-11: Explicittext-whiteclass applied for contrast
Addingtext-whiteensures the index label remains legible against the pink background.
15-15: Use oftext-al-pinktoken for the tick icon
Thetext-al-pinktoken aligns with our design system’s color palette.libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html (1)
5-6: Footer logo and navigation setup looks good
Explicitly centering the logo and passingwhiteFont="true"to<al-navigation>ensures consistent styling.libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.scss (2)
15-17: Updated default grid-template-areas
The new two-row layout ('logo social'/'nav nav') is clear and concise for smaller viewports.
19-23: Verify breakpoint rename from@screen xlto@screen lg
Dropping the MD breakpoint and shifting the XL layout to LG may alter mid-size layouts. Please confirm that the updated breakpoints align with the design mockups and QA in all viewport widths.libs/shared/assets/src/lib/styles/main.scss (2)
11-17: New CSS variables for theming added
Introducing--primary-foregroundand--greycompletes the palette for both light and dark modes. This aligns with the Tailwind configuration’s new tokens.
19-29: Light-mode media query block implemented
Theprefers-color-scheme: lightoverride correctly defines lighter values for all variables, enabling automatic theme switching.libs/blog/shared/ui-card/src/lib/card.component.ts (2)
71-73: GradientCardDirective classes updated for theme variants
Switching to'bg-al-radial-gradient dark:bg-al-background'removes the border and adds a dark-mode fallback—this matches our updated theming strategy.
99-101: Ensure component and directivehostClassesmerge correctly
BothCardComponentand a consumer ofalGradientCardbind to theclassattribute. Please verify in the rendered DOM that classes from both the component and the directive are applied and not overridden.libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (3)
11-11: Great conversion to standalone component!This conversion to a standalone component aligns with modern Angular best practices and improves the component's modularity.
23-24: Properly implemented light/dark mode stylingThe
alGradientCarddirective and conditional dark/light mode classes are correctly applied, ensuring consistent styling that adapts to the theme. The background styling appropriately uses different colors for each theme variant.
27-28: LGTM: Content styling with directiveAdding the
alCardContentdirective to the inner content maintains styling consistency with other card components in the application.libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (3)
3-3: Improved positioning and container stylingThe addition of
relative,overflow-hidden, andbg-blackclasses properly prepares the container for the new image layering approach.
6-13: Great improvement using proper image elementsReplacing CSS background with an actual
<img>element is a significant improvement that:
- Enhances accessibility with proper alt text
- Enables image optimization through NgOptimizedImage
- Provides better loading control with the priority attribute
- Makes explicit width/height handling possible
The absolute positioning with opacity control creates a nice background effect while maintaining content readability.
14-14: Proper text color handlingThe explicit text color setting ensures good contrast over the background image in all scenarios.
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (2)
1-1: Well-implemented standalone component conversionThe component has been properly converted to standalone with all necessary imports. Including NgOptimizedImage enables better image loading performance and optimization.
Also applies to: 13-20
25-25: Good addition of image priority controlThe new
imagePriorityinput property allows control over image loading priority, which is important for performance optimization and core web vitals. This aligns with the template changes using this property.libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (5)
3-3: Improved border styling for theme consistencyThe border styling now properly handles light and dark mode variations, maintaining visual separation in light mode while using transparent borders in dark mode.
21-25: Enhanced avatar component with priority loadingAdding the
[priority]binding to the avatar component improves loading performance for this important UI element.
26-28: Proper text color styling for author nameThe explicit text color class ensures consistent styling across light and dark modes.
41-47: Improved heading styling with proper color and text propertiesThe heading now uses explicit color, font weight, and style classes, ensuring consistent appearance across theme modes.
48-50: Consistent paragraph styling with theme-aware colorsThe paragraph styling now properly uses theme-specific colors with appropriate font styling.
tailwind.preset.js (4)
1-5: Excellent addition of theme variant supportGood job introducing the
tailwindcss-theme-variantsplugin which enables media query-based theme switching. This is a more modern approach than the class-based dark mode.
13-14: New color tokens look goodThe addition of 'al-primary-foreground' and 'al-grey' tokens will help maintain consistent theming across light and dark modes.
Also applies to: 22-22
35-35: Box shadow color change looks intentionalChanging from
--borderto--greyfor theal-full-bordershadow matches the new theming approach. Just ensure this change doesn't unexpectedly affect existing components.
39-51: Well-structured theme configurationThe plugin configuration properly sets up light and dark themes using media queries. This approach allows the UI to automatically adapt to the user's system preferences.
libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts (3)
28-30: Good adaptation for light/dark mode in left blockThe conditional class application correctly handles both light and dark modes based on the
isColorBackground()condition:
- Using
dark:bg-al-background bg-transparentfor colored backgrounds- Using
bg-al-greyfor non-colored backgroundsThis approach maintains visual consistency across themes.
47-50: Well-implemented shadow handling for different themesThe shadow classes are properly updated for the dot container:
dark:shadow-al-full-background shadow-transparentfor colored backgroundsshadow-al-full-borderfor non-colored backgroundsThis ensures proper visual representation in both light and dark modes.
80-82: Consistent styling in right blockThe right block styling matches the left block's implementation, maintaining design consistency throughout the component. The code cleanly handles both light and dark themes.
| <h3 | ||
| id="social-media-title" | ||
| class="mb-4 hidden text-sm font-bold lg:block" | ||
| class="mb-4 hidden text-sm font-bold text-[#fff] lg:block" |
There was a problem hiding this comment.
Use design token instead of hardcoded hex
To support dynamic theming and maintain consistency with other components, replace the literal hex color with the text-al-primary-foreground token:
- class="mb-4 hidden text-sm font-bold text-[#fff] lg:block"
+ class="mb-4 hidden text-sm font-bold text-al-primary-foreground lg:block"📝 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.
| class="mb-4 hidden text-sm font-bold text-[#fff] lg:block" | |
| class="mb-4 hidden text-sm font-bold text-al-primary-foreground lg:block" |
| variants: { | ||
| variant: <Record<AlPillVariant, string>>{ | ||
| flat: 'bg-al-primary/90', | ||
| flat: 'bg-al-primary/90 text-white', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use design token for pill text color
For theme variant compatibility, replace text-white with the text-al-primary-foreground token in the flat variant:
- flat: 'bg-al-primary/90 text-white',
+ flat: 'bg-al-primary/90 text-al-primary-foreground',📝 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.
| flat: 'bg-al-primary/90 text-white', | |
| flat: 'bg-al-primary/90 text-al-primary-foreground', |
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html
Show resolved
Hide resolved
| }) | ||
| export class NavigationComponent { | ||
| readonly layout = input<'vertical' | 'horizontal'>('horizontal'); | ||
| readonly whiteFont = input<boolean>(false); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider a more flexible theming approach instead of a boolean flag
While adding a whiteFont input provides basic text color control, a more comprehensive theming approach would better support light mode implementation.
Consider using a theme enum or string input that supports multiple theme options:
- readonly whiteFont = input<boolean>(false);
+ readonly textTheme = input<'default' | 'light' | 'dark' | 'contrast'>('default');Then in the template, you could apply different classes based on the selected theme:
[ngClass]="{
'text-white': textTheme() === 'light',
'text-al-foreground': textTheme() === 'default',
'text-al-primary': textTheme() === 'dark',
'text-al-contrast': textTheme() === 'contrast'
}"This approach would be more maintainable as you add more theme variants beyond just light mode.
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html
Outdated
Show resolved
Hide resolved
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html
Outdated
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (1)
42-42: Consider addressing the architectural TODO.The TODO comment highlights a valid architectural concern about decoupling the store from
CCAppThemeConsumer. Consider creating an interface to improve testability and modularity.Would you like me to help design an interface to decouple the theme store from the consumer implementation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/blog/src/assets/icons/moon.svgis excluded by!**/*.svgapps/blog/src/assets/icons/sun.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts(3 hunks)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts(3 hunks)libs/blog/shared/ui-icon/src/lib/icon/icon.component.ts(1 hunks)libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts(4 hunks)libs/shared/assets/src/lib/styles/main.scss(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/blog/shared/ui-icon/src/lib/icon/icon.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/shared/assets/src/lib/styles/main.scss
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (1)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (1)
AppThemeStore(11-34)
🔇 Additional comments (11)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (4)
3-3: Good addition of patchState import.The import of
patchStateis necessary for properly updating the signal store state in the new methods.
5-5: Good export of Theme type for reusability.Exporting the
Themetype allows other components to import and use the same type definition, promoting type consistency across the application.
27-31: Excellent implementation of theme toggle functionality.The
toggleThememethod correctly:
- Toggles between 'dark' and 'light' themes
- Updates the DOM attribute via the consumer
- Patches the store state with the new theme
The logic is clean and follows the signal store pattern properly.
45-47: Improvement: Using data attributes instead of CSS classes.The change from
setThemeClasstosetThemeAttributeis a good architectural improvement. Usingdata-themeattributes instead of CSS classes provides better separation of concerns and is more semantic for theme management.libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (3)
79-81: Clean integration of theme store.The injection of
AppThemeStoreand creation of a computedthemesignal provides reactive theme state to the template. This follows Angular best practices for signal-based state management.
31-33: Proper template integration for theme management.The template correctly:
- Passes the reactive
theme()signal to the header component- Handles the
themeToggleevent to trigger theme switchingThis creates a clean data flow from store → shell → header.
104-106: Simple and effective theme toggle handler.The
onThemeTogglemethod correctly delegates to the store'stoggleThememethod, maintaining separation of concerns between UI events and state management.libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (4)
83-83: Well-typed theme input.The
themeinput is properly typed as'light' | 'dark'and marked as required, ensuring type safety and preventing undefined theme states.
87-87: Clean event-driven architecture.The
themeToggleoutput event follows Angular patterns for component communication, allowing the parent component to handle theme changes appropriately.
91-93: Intuitive icon selection logic.The
themeSwitchIconcomputed property correctly shows:
- Moon icon in light mode (to indicate switching to dark)
- Sun icon in dark mode (to indicate switching to light)
This provides clear visual indication of the toggle action.
41-52: Excellent accessibility and styling for theme toggle.The theme toggle button implementation includes:
- Proper
aria-labelfor screen readers- Clean styling with transparent background
- Proper click event handling
- Test ID for automated testing (once typo is fixed)
- Semantic use of SVG icons
This provides a great user experience for theme switching.
libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts
Outdated
Show resolved
Hide resolved
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tailwind.preset.js (1)
1-1: Remove unused import or utilize the imported themeVariants.The
themeVariantsimport from 'tailwindcss-theme-variants' is not being used in the plugins array. You're implementing a custom plugin instead.Apply this diff to remove the unused import:
-const { themeVariants } = require('tailwindcss-theme-variants');Alternatively, if you intend to use the library's implementation, replace the custom plugin with:
-const { themeVariants } = require('tailwindcss-theme-variants'); +const { themeVariants } = require('tailwindcss-theme-variants'); // In plugins array: - function ({ addVariant }) { - addVariant( - 'light', - '&:where([data-theme="light"], [data-theme="light"] *)', - ); - addVariant('dark', '&:where([data-theme="dark"], [data-theme="dark"] *)'); - }, + themeVariants({ + themes: { + light: 'selector', + dark: 'selector' + } + }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html(2 hunks)libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.ts(2 hunks)libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html(1 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts(1 hunks)libs/blog/shared/ui-button/src/lib/button/button.component.ts(1 hunks)libs/blog/shared/ui-card/src/lib/card.component.ts(2 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html(1 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts(2 hunks)libs/shared/assets/src/lib/styles/main.scss(2 hunks)tailwind.preset.js(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html
- libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- libs/blog/shared/ui-button/src/lib/button/button.component.ts
- libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts
- libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html
- libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html
- libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html
- libs/shared/assets/src/lib/styles/main.scss
- libs/blog/shared/ui-card/src/lib/card.component.ts
- libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.ts (1)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (1)
AppThemeStore(11-38)
🔇 Additional comments (10)
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts (3)
4-6: LGTM: Modern Angular signal importsGood adoption of Angular's signal-based reactive programming with
computedandinput.
15-15: LGTM: Well-defined variant typeThe type definition provides good type safety for the theming variants.
27-27: LGTM: Modern input signal usageUsing Angular's signal-based input with a sensible default value follows current best practices.
libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.ts (3)
19-19: LGTM! Clean integration with the theme system.The import correctly adds the
AppThemeStorefollowing the established import pattern and module structure.
45-45: Excellent use of modern Angular patterns.The injection uses the proper
inject()function and correctly accesses the theme signal from the store. Thereadonlymodifier appropriately prevents direct modification.
47-47: Well-implemented theme-aware computed property.The computed property correctly implements the gradient hiding logic for light mode using Angular's reactive patterns. The naming is descriptive and the logic is straightforward and appropriate.
tailwind.preset.js (4)
5-5: LGTM: Appropriate darkMode configuration for attribute-based theming.The change from
['class']to'selector'correctly aligns with the data-theme attribute approach used throughout the application.
10-11: LGTM: New color token follows established pattern.The
al-primary-foregroundcolor token is properly defined using the CSS variable pattern with alpha value support.
19-19: LGTM: Grey color token addition supports theming system.The
al-greycolor token follows the established pattern and will be used in the updated box shadow configuration.
32-32: LGTM: Box shadow color update aligns with new color tokens.The change from
--borderto--greyCSS variable correctly utilizes the newly addedal-greycolor token.
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (3)
3-3: Theme token instead of hard-coded blackReplace bg-black with a theme-aware token (e.g., bg-al-card) to support light/dark modes.
6-13: Absolute image fit/positioning and boolean priorityUse object-fit utilities and explicit positioning (e.g., absolute inset-0 h-full w-full object-cover) and coerce priority to boolean: [priority]="!!imagePriority()".
14-14: Text color should use theme token; ensure stackingPrefer text-al-foreground (or equivalent) over text-[#fff] and ensure content has higher z-index than overlay/image.
tailwind.preset.js (2)
53-61: Remove redundant custom dark variant or align with darkModeAvoid overlapping Tailwind’s built-in dark:. Either keep darkMode and only add a custom light: variant, or adopt themeVariants plugin and drop darkMode/manual variants.
3-3: Fix darkMode configurationdarkMode: 'selector' is not valid alone. Use darkMode: ['selector', '[data-theme="dark"]'] or revert to 'class'/'media'.
🧹 Nitpick comments (2)
libs/blog/shared/ui-button/src/lib/button/button.component.ts (1)
24-24: Use theme token for text color instead of hard-coded whitePrefer text-al-primary-foreground to match the new tokenized theming.
Apply:
- Primary: 'bg-al-primary/90 text-white uppercase', + Primary: 'bg-al-primary/90 text-al-primary-foreground uppercase',libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (1)
24-24: Clarify priority typing vs NgOptimizedImage booleanNgOptimizedImage [priority] expects boolean; imagePriority is number|null. Ensure template coerces to boolean (e.g., [priority]="!!imagePriority()") or change the input type to boolean if that matches usage across variants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/blog/src/assets/i18n/en.json(1 hunks)apps/blog/src/assets/i18n/pl.json(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts(2 hunks)libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts(4 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html(1 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts(2 hunks)libs/blog/newsletter/feature-newsletter/src/lib/page/newsletter-examples/newsletter-examples.component.ts(0 hunks)libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-additional-description/roadmap-dialog-additional-description.component.ts(1 hunks)libs/blog/roadmap/ui-roadmap/src/lib/components/vertical-connector-arrow/vertical-connector-arrow.component.ts(1 hunks)libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts(1 hunks)libs/blog/shared/ui-button/src/lib/button/button.component.ts(1 hunks)tailwind.preset.js(3 hunks)
💤 Files with no reviewable changes (1)
- libs/blog/newsletter/feature-newsletter/src/lib/page/newsletter-examples/newsletter-examples.component.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-additional-description/roadmap-dialog-additional-description.component.ts
- libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html
- libs/blog/roadmap/ui-roadmap/src/lib/components/vertical-connector-arrow/vertical-connector-arrow.component.ts
- libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts
🧰 Additional context used
🧬 Code graph analysis (3)
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (2)
libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.ts (1)
UiArticleCardComponent(26-30)libs/blog/shared/ui-card/src/lib/card.component.ts (2)
GradientCardDirective(69-74)CardComponent(94-101)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (5)
libs/blog/shared/types/src/lib/article-card.ts (1)
ArticleCard(1-13)libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.ts (1)
ArticleRegularCardComponent(22-25)libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.ts (1)
UiArticleCardComponent(26-30)libs/blog/shared/ui-avatar/src/lib/avatar.component.ts (1)
AvatarComponent(13-17)libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.ts (1)
ArticleHorizontalCardComponent(36-52)
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (2)
libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.ts (1)
ArticleRegularCardComponent(22-25)libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.ts (1)
UiArticleCardComponent(26-30)
🔇 Additional comments (10)
libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts (1)
17-17: LGTM! Typo fixed.The attribute name has been corrected from
date-testidtodata-testid, aligning with the HTML5 data attribute convention and ensuring test selectors function properly.libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (3)
1-1: LGTM!The added imports (
computed,input) are correctly used throughout the component for defining signals and computed properties.
19-32: LGTM!The template structure correctly uses dynamic class bindings with computed properties to adjust layout based on the
articleCardinput. The responsive design patterns withmd:breakpoints are appropriate.
16-16: GradientCardDirective already defineshideGradientas an input
The directive in libs/blog/shared/ui-card/src/lib/card.component.ts (line 73) useshideGradient = input<boolean>(false), so the[hideGradient]binding is valid.libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts (1)
47-48: LGTM!The new
hideGradientandarticleCardinputs are correctly implemented using Angular's signal-based input API. The default values are appropriate for the parent component context.libs/blog/shared/ui-button/src/lib/button/button.component.ts (1)
20-20: Alignment improvement looks goodAdding justify-center is a sensible default for consistent horizontal alignment.
apps/blog/src/assets/i18n/pl.json (1)
31-31: Translation addition LGTMtoggle_theme key added and aligned with EN.
Confirm it’s used for visible label and aria-label/title in the theme toggle control.
apps/blog/src/assets/i18n/en.json (1)
31-31: Translation addition LGTMKey is well placed under nav and aligns with PL.
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (1)
13-19: NgOptimizedImage integration looks goodIncluding NgOptimizedImage in imports is appropriate for optimized loading.
tailwind.preset.js (1)
11-12: CSS variables exist; al-roadmap-gradient is static
CSS custom properties--primary-foreground,--grey, and--footer-backgroundare defined in libs/shared/assets/src/lib/styles/main.scss;al-roadmap-gradientuses a hardcoded radial gradient and doesn’t require a CSS var.
| readonly hideGradient = input<boolean>(true); | ||
| readonly articleCard = input<boolean>(false); |
There was a problem hiding this comment.
Inconsistent default values between parent and template components.
The hideGradient input defaults to true here but defaults to false in the parent AuthorCardComponent (line 47 of author-card.component.ts). This mismatch could lead to unexpected behavior when the parent doesn't explicitly pass the value.
Align the defaults to match the parent component:
- readonly hideGradient = input<boolean>(true);
+ readonly hideGradient = input<boolean>(false);📝 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.
| readonly hideGradient = input<boolean>(true); | |
| readonly articleCard = input<boolean>(false); | |
| readonly hideGradient = input<boolean>(false); | |
| readonly articleCard = input<boolean>(false); |
🤖 Prompt for AI Agents
In
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
around lines 41 to 42, the hideGradient input currently defaults to true which
mismatches the parent AuthorCardComponent (author-card.component.ts line 47)
that defaults it to false; change the hideGradient input default to false so
both components share the same default behavior, and verify there are no other
input default mismatches between the template and parent.
| protected readonly authorInfoCardClass = computed(() => | ||
| !this.articleCard() | ||
| ? 'md:border dark:!bg-al-radial-gradient dark:bg-al-background md:light:bg-[#f2f2f2] pb-6' | ||
| : '', | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify Tailwind configuration for light mode variant and avoid !important.
Line 50 contains two potential issues:
- The
!importantflag indark:!bg-al-radial-gradientmight indicate a specificity issue that should be resolved through proper CSS architecture rather than forcing precedence. - The
md:light:bg-[#f2f2f2]uses alight:variant that may not be a standard Tailwind feature. Verify this variant is properly configured in your Tailwind setup.
Run the following script to check your Tailwind configuration:
Additionally, search the web to confirm whether light: is a valid Tailwind CSS variant in your version:
🏁 Script executed:
#!/bin/bash
# Description: Check Tailwind configuration for light mode variant
# Search for Tailwind config files
fd -e js -e ts 'tailwind.config'
# Search for light mode variant configuration
rg -nP --type=ts --type=js -C5 'variant.*light|darkMode|class.*strategy'
# Check if light: is used elsewhere in the codebase
rg -nP --type=ts --type=html 'light:' -g '!node_modules'Length of output: 1856
🌐 Web query:
Is "light:" a valid Tailwind CSS variant for light mode styling? How should light mode be implemented with Tailwind's dark mode class strategy?
💡 Result:
Short answer: No — Tailwind does not include a built‑in light: variant. Tailwind exposes a dark: variant (and the unprefixed classes act as the “light” / default styles). Configure dark mode with darkMode: 'media' or darkMode: 'class' and toggle by adding/removing a .dark parent (commonly on ). Example:
-
tailwind.config.js
- darkMode: 'class' // or 'media'. [1]
-
HTML usage (class strategy)
-
…
If you want an explicit light: variant, add a custom variant (Tailwind v4 supports @custom-variant) or use a selector like html:not(.dark) with an arbitrary selector utility. Example custom variant pattern shown in community answers:
- @custom-variant light (html:not(.dark) &); /* then use light:bg-red-500 */. [2]
Sources: Tailwind dark mode docs and community examples. [1][2]
References:
[1] Tailwind CSS — Dark Mode docs.
[2] StackOverflow: “TailwindCSS: How to do a light mode only modification?”
Avoid !important and define or remove light: variant
Remove the ! in dark:!bg-al-radial-gradient and handle specificity via Tailwind utilities or CSS. Since Tailwind has no built-in light: variant, add a custom light variant in your config (e.g., using @custom-variant or html:not(.dark)) or revert to unprefixed classes for light mode.
🤖 Prompt for AI Agents
In
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
around lines 48 to 52, remove the `!` (important) from
`dark:!bg-al-radial-gradient` and replace it with a normal
`dark:bg-al-radial-gradient` (or refactor to use a higher-specificity
utility/CSS rule) and either remove the unsupported `light:` prefix from the
light-mode class or add a custom `light` variant in the Tailwind config (e.g.,
define a `light` variant using `html:not(.dark)` or `@custom-variant`) so
light-mode styles are applied correctly; ensure specificity is handled via
Tailwind utilities or custom CSS rather than using `!important`.
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts
Outdated
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
| return this.authorsCards()?.length || 0; | ||
| }); | ||
|
|
||
| readonly theme = inject(AppThemeStore).theme; |
libs/blog/articles/feature-comments/src/lib/giscus-comments/giscus-comments.component.ts
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
| switch (this.variant()) { | ||
| case 'primary': | ||
| return 'm-[2px] bg-al-roadmap-primary text-[24px]'; | ||
| return 'm-[2px] bg-al-roadmap-primary light:bg-[#edcafa] text-[24px]'; |
There was a problem hiding this comment.
może coś zamiast light:bg-[#edcafa]
| }, | ||
| }) | ||
| export class GradientCardDirective { | ||
| hideGradient = input<boolean>(false); |
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/blog/shared/ui-card/src/lib/card.component.ts (1)
103-103: Consider theming CardSkeletonComponent background
CardSkeletonComponent still usesbg-white(libs/blog/shared/ui-card/src/lib/card-skeleton.component.ts:38); consider applying theme-aware classes likebg-al-card/dark:bg-al-backgroundto match CardComponent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/blog/shared/ui-card/src/lib/card.component.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
libs/blog/shared/ui-card/src/lib/card.component.ts (1)
libs/blog/shared/ui-card/src/lib/card-skeleton.component.ts (1)
CardSkeletonComponent(34-39)
🔇 Additional comments (1)
libs/blog/shared/ui-card/src/lib/card.component.ts (1)
68-76: LGTM! Clean implementation of conditional gradient control.The refactoring correctly implements the gradient visibility feature:
- The
hideGradientinput with proper signal syntax and default value- Dynamic class binding (
!hideGradient()) applies the gradient by default- Simplified
hostClassesto only theme-specific styling (dark:bg-al-background)
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
libs/blog/articles/feature-article/src/lib/article-share-icons/article-share-icons.component.ts (1)
40-62: Remove invalidtargetattribute from button.The semantic change from
<a>to<button>is correct for a non-navigational action (copying to clipboard). However, thetarget="_blank"attribute on line 46 is invalid HTML—targetis only valid on<a>and<form>elements. This appears to be leftover from the anchor element and serves no purpose on a button.Apply this diff to remove the invalid attribute:
<button [attr.aria-label]="t('articleShareIcons.urlAriaLabel')" [class.url-icon-animated]="animating()" [cdkCopyToClipboard]="articleUrl()" (click)="animating.set(true)" (animationend)="animating.set(false)" - target="_blank" >libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-footer/roadmap-dialog-footer.component.ts (1)
53-74: Translate aria-label values for share icons
TheariaLabelprops inlibs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-footer/roadmap-dialog-footer.component.ts(lines 61, 66, 71) are raw keys with no translation logic or entries; wrap them in a Transloco pipe or usetranslocoService.translate()and add the corresponding keys to your JSON files.libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card.component.html (1)
4-7: Broken fallback for background-image due to operator precedence.'url(' + article().featuredImageUrl || 'assets/article-placeholder.webp' + ')' builds 'url(undefined)' when the URL is falsy; || never triggers. Parenthesize the fallback.
Apply this diff:
- [style.background-image]=" - 'url(' + article().featuredImageUrl || - 'assets/article-placeholder.webp' + ')' - " + [style.background-image]=" + 'url(' + (article().featuredImageUrl || 'assets/article-placeholder.webp') + ')' + "libs/blog/shared/ui-social-media-icons/src/lib/social-media-icon-item.component.ts (1)
34-45: Add rel="noopener noreferrer" on external linkPrevents reverse tabnabbing and improves security for target="_blank".
- <a + <a *transloco="let t" class="mr-2 flex" [attr.aria-label]="t(socialMediaConfig().ariaLabel)" [href]="href" target="_blank" + rel="noopener noreferrer" > <fast-svg aria-hidden="true" [name]="socialMediaConfig().iconName" size="24" />Optionally add focus suppression to the rendered SVG if supported by FastSvg (e.g., [attr.focusable]="false").
libs/blog/layouts/ui-layouts/src/lib/header/components/header-logo.component.ts (1)
8-31: Addstandalone: true(required for usage viaimports)This component is consumed via
importsin HeaderComponent; it must be standalone.@Component({ selector: 'al-header-logo', + standalone: true, template: ` <a *transloco="let t; read: 'nav'" data-testid="header-home" class="flex items-center gap-2" [attr.aria-label]="t('home')" [routerLink]="'/' | alLocalize" >libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts (1)
16-42: Addstandalone: true(required for usage viaimports)Used in HeaderComponent’s
imports. Mark as standalone.@Component({ selector: 'al-header-mobile-menu', + standalone: true, template: ` <div class="bg-al-background fixed -top-[100%] left-0 z-40 flex h-full w-full flex-row items-center justify-center transition-transform duration-300 lg:hidden" [ngClass]="{ 'translate-y-[100%]': isOpened() }" - [attr.aria-hidden]="isOpened()" + [attr.aria-hidden]="!isOpened()" >
♻️ Duplicate comments (1)
libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (1)
2-2: Use theme variable-based border instead of hard-coded Tailwind border.Stick to the app’s theme-driven border utility (e.g., border-al-border) for consistency across light/dark, rather than border border-transparent.
Example:
- class="... rounded-lg border border-transparent shadow-none ..." + class="... rounded-lg border-al-border shadow-none ..."
🧹 Nitpick comments (22)
libs/blog/newsletter/feature-newsletter/src/lib/page/newsletter-succes/newsletter-succes.component.html (1)
7-7: Good a11y improvement; consider also preventing focus on the SVG.aria-hidden="true" is correct for a decorative icon. Optionally add focusable="false" to avoid legacy browsers focusing the SVG.
- <fast-svg name="circle-check" aria-hidden="true" size="80" /> + <fast-svg name="circle-check" aria-hidden="true" focusable="false" size="80" />If this is @push-based/ngx-fast-svg, attributes should pass through to the rendered SVG. Based on learnings
libs/blog/search/ui-search-result/src/lib/search-result-item/search-result-item.component.html (1)
18-24: Solid a11y upgrade; add UTC todatetimeto avoid SSR/locale driftGreat:
<time datetime>added and the clock icon isaria-hidden.To prevent server/client hydration mismatches and cross‑timezone date shifts, set the
datepipe timezone to UTC for the machine‑readabledatetimevalue (keep the visible text localized as-is).Apply this minimal change:
- [attr.datetime]="item.publishDate | date: 'yyyy-MM-dd'" + [attr.datetime]="item.publishDate | date: 'yyyy-MM-dd':'UTC'"Optional: Improve the reading-time announcement for screen readers (e.g., wrap “{{ item.readingTime }} min” with
aria-label="{{ item.readingTime }} minute read").libs/blog/partners/ui-partners/src/lib/partners/partners.component.html (1)
5-7: Avoid duplicate IDs and restore heading typography.
- The hardcoded id risks collisions when multiple instances are on a page; bind a unique id and reference it from aria-labelledby.
- Heading lost its previous size/weight; re-apply DS typography classes to avoid a visual regression.
Apply in template:
-<section - aria-labelledby="partners-heading" +<section + [attr.aria-labelledby]="headingId" class="flex w-full flex-col justify-between rounded-lg bg-transparent" > - <h2 id="partners-heading" *transloco="let t" class="mb-4"> + <h2 [attr.id]="headingId" *transloco="let t" class="mb-4 text-2xl font-bold"> {{ t(heading()) }} </h2>And in partners.component.ts (outside this diff), add:
// inside class private static _next = 0; readonly headingId = `partners-heading-${PartnersComponent._next++}`;Optional: if rounded corners aren’t visible without a background/border, consider removing
rounded-lg.libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html (3)
14-14: Prefer theme token for “on-pink” text instead of hard-coded whiteTo keep theming flexible, consider using your design token (e.g., text-al-on-pink) instead of text-white. This avoids hard-coding if the pink shade changes in light/dark themes.
20-25: Good: decorative icon is hidden from AT; add toggle semantics for the headeraria-hidden="true" on the icon is correct. Also make the header toggle accessible:
- Add role="button", tabindex="0", and [attr.aria-expanded]="isOpen()"
- Handle Enter/Space key to toggle
Example (outside selected lines):
<div class="flex w-full cursor-pointer justify-between" role="button" tabindex="0" [attr.aria-expanded]="isOpen()" (keydown.enter)="toggleStep.emit(index())" (keydown.space)="toggleStep.emit(index())" (click)="toggleStep.emit(index())" (transitionend)="scrollToRule.emit(index())" >
47-53: Consistent theming + focus styles for buttonsGood call adding text-white for the pink “Next” button. For consistency in light mode:
- Ensure “Previous” button text has an explicit, theme-aware color (not default) for contrast.
- Add focus-visible styles (e.g., focus-visible:outline or ring) if not provided globally.
- Consider extracting a “primary/secondary” button class/variant to avoid repeating long class lists.
libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-footer/roadmap-dialog-footer.component.ts (1)
21-33: Consider removingrole="button"from the anchor element.The anchor element has
role="button"(line 22) but functions as a link with anhrefattribute. Links and buttons have different semantics and keyboard behavior expectations. Since this element navigates to a URL, it should remain a semantic link without the button role.Apply this diff to remove the incorrect role:
<a - role="button" [attr.aria-label]="item.ariaLabel" [href]="item.href" target="_blank" >libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html (4)
3-5: Trailing comma in [ngClass] is fine; align with formatter/lint rules.No functional issue. Ensure your HTML/Angular template formatter and ESLint/prettier config allow trailing commas in template expressions to avoid churn.
11-11: Avoid hard‑coded text-white; prefer a theme/semantic token for light/dark.On a themed badge (bg-al-pink), use a semantic foreground (e.g., “on‑pink”/contrast) or inherit currentColor from the container to keep contrast correct across themes.
15-15: Good a11y: aria-hidden for decorative SVG.Nice improvement. Optionally ensure it can’t be focused (fast-svg may already output non-focusable SVGs; if not, expose focusable="false").
1-6: List semantics: ensure this renders inside a real list or add role.If this is used outside
/
, consider role="listitem" (or switch to
- ) for assistive tech semantics.
libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html (1)
1-4: Prefer aria-labelledby (or no label) over aria-label on link.The link already contains the visible title; aria-label can override and cause duplicate/unnatural announcements. Use [attr.aria-labelledby]="article().slug" or remove the label.
Apply one of:
-[attr.aria-label]="article().title" +[attr.aria-labelledby]="article().slug"or
-[attr.aria-label]="article().title"libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (1)
5-7: Prefer aria-labelledby (or remove) instead of aria-label on link.The title is present; aria-label may override and duplicate. Reference the heading id or omit.
-[attr.aria-label]="article().title" +[attr.aria-labelledby]="article().slug"or remove the attribute.
libs/blog/shared/ui-focusable/src/lib/focusable-item.directive.ts (2)
16-18: Consider accepting the optional FocusOrigin parameter.The
FocusableOptioninterface definesfocus(origin?: FocusOrigin): void, but this implementation omits the optional parameter. While TypeScript allows this, explicitly accepting and ignoring the parameter would better match the interface contract and improve compatibility with CDK's FocusKeyManager.Apply this diff to match the interface signature:
- focus(): void { + focus(_origin?: FocusOrigin): void { this._elementRef.nativeElement.focus(); }Remember to import FocusOrigin:
-import { FocusableOption } from '@angular/cdk/a11y'; +import { FocusableOption, FocusOrigin } from '@angular/cdk/a11y';
4-19: Ensure host element is focusable.The directive calls
focus()on the native element but doesn't guarantee the element is focusable (e.g., non-interactive elements like<div>needtabindexto be focusable). Consider adding a host binding to ensure focusability.Apply this diff to add tabindex handling:
@Directive({ selector: '[alFocusableItem]', + host: { + '[attr.tabindex]': 'focusDisabled() ? -1 : 0', + }, })This ensures the element can receive focus programmatically and sets
tabindex="-1"when disabled to prevent keyboard navigation to disabled items.libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (2)
92-95: Expose the store signal directly (avoid redundant computed)Wrap not needed; use the store’s signal as-is.
- private readonly _appThemeStore = inject(AppThemeStore); - - protected readonly theme = computed(() => this._appThemeStore.theme()); + private readonly _appThemeStore = inject(AppThemeStore); + protected readonly theme = this._appThemeStore.theme;
122-124: Initialize theme on app start to avoid flash and pick persisted/system valueCall syncWithSystemTheme once on bootstrap (SSR-safe in store).
You can append this in the constructor after getData():
this._appThemeStore.syncWithSystemTheme();libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.scss (1)
1-4: Use max-height + overflow for better fit and scrollingFixed height can clip content. Prefer max-height and enable scrolling. Optionally use dvh.
.mobile-menu__navigation { - height: min(100%, 30rem); + max-height: min(100%, 30rem); + overflow: auto; padding-block: 2rem; }Optional: replace 100% with 100dvh if this element is sized to viewport.
libs/blog/home/feature-home/src/lib/home-page/home-page.component.ts (1)
2-2: Remove unused ReactiveFormsModuleNo form controls in the template; drop the import to reduce bundle size.
-import { ReactiveFormsModule } from '@angular/forms'; @@ imports: [ - ReactiveFormsModule, FeatureLatestArticlesComponent, ArticlesListContainerComponent, WelcomeMessageComponent, ],Also applies to: 11-16
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html (2)
12-14: Unify padding binding; remove redundant md:p-6Static md:p-6 conflicts with conditional [class.md:p-6]. Rely on the binding only.
- class="text-al-foreground p-2 font-medium md:p-6" + class="text-al-foreground p-2 font-medium" [class.md:p-6]="!cols() && layout() === 'horizontal'"
7-7: Use stable track expression (not index)Reduces DOM churn on reorders/updates.
- @for (item of navItems(); track $index) { + @for (item of navItems(); track item.dataTestId) {libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.ts (1)
50-101: Consider centralizing navigation item definitions if overlap is significant.The breakpoint observer setup is correct for responsive behavior. However, the footer defines its own
navItemsarray (8 items) which has some overlap withheaderNavItems(4 items from header-nav-items.ts):Common items between header and footer:
- guides, news, meetups, roadmap (in header) vs guides, news, meetups (in footer)
Footer-only items:
- about, become_author, newsletter, in_depth, partnership
If this separation is intentional (different navigation for header vs footer), this is fine. However, if there are shared items, consider extracting common definitions to reduce duplication and ensure consistency.
Apply this refactor if navigation items should be centralized:
Option 1: Create shared navigation config file
Create
libs/blog/layouts/ui-navigation/src/lib/navigation-config.ts:export const commonNavItems = { guides: { translationPath: 'nav.guides', link: ['guides'], dataTestId: 'navigation-guides', }, news: { translationPath: 'nav.news', link: ['news'], dataTestId: 'navigation-news', }, meetups: { translationPath: 'nav.meetups', link: ['https://meetup.angular.love/'], externalLink: true, dataTestId: 'navigation-meetups', }, // ... other common items } as const; export const headerNavItems: NavItem[] = [ commonNavItems.guides, commonNavItems.news, commonNavItems.meetups, // ... header-specific items ]; export const footerNavItems: NavItem[] = [ commonNavItems.guides, // ... footer-specific items ];Then update the footer to import
footerNavItemsinstead of defining inline.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/blog/src/assets/HOA_logo_blue.pngis excluded by!**/*.pngapps/blog/src/assets/HOA_logo_white.pngis excluded by!**/*.png
📒 Files selected for processing (54)
apps/blog/src/assets/i18n/en.json(3 hunks)apps/blog/src/assets/i18n/pl.json(3 hunks)libs/blog/articles/feature-article/src/lib/article-details/article-details.component.html(2 hunks)libs/blog/articles/feature-article/src/lib/article-share-icons/article-share-icons.component.ts(2 hunks)libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html(2 hunks)libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts(5 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html(2 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card.component.html(1 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html(2 hunks)libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html(3 hunks)libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html(1 hunks)libs/blog/home/feature-home/src/lib/home-page/home-page.component.html(1 hunks)libs/blog/home/feature-home/src/lib/home-page/home-page.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.scss(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/footer/partners.ts(0 hunks)libs/blog/layouts/ui-layouts/src/lib/header/components/header-hamburger.component.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/header/components/header-logo.component.ts(2 hunks)libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.scss(1 hunks)libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts(2 hunks)libs/blog/layouts/ui-layouts/src/lib/header/header-nav-items.ts(1 hunks)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts(4 hunks)libs/blog/layouts/ui-navigation/src/lib/language-picker/language-picker.component.ts(3 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html(2 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts(1 hunks)libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html(5 hunks)libs/blog/newsletter/feature-newsletter/src/lib/page/newsletter-succes/newsletter-succes.component.html(1 hunks)libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html(2 hunks)libs/blog/partners/ui-partners/src/lib/partners/partners.component.html(1 hunks)libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-footer/roadmap-dialog-footer.component.ts(1 hunks)libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts(1 hunks)libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.html(2 hunks)libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.ts(0 hunks)libs/blog/search/ui-search-result/src/lib/search-result-item/search-result-item.component.html(1 hunks)libs/blog/shared/ui-focusable/.eslintrc.json(1 hunks)libs/blog/shared/ui-focusable/README.md(1 hunks)libs/blog/shared/ui-focusable/jest.config.ts(1 hunks)libs/blog/shared/ui-focusable/project.json(1 hunks)libs/blog/shared/ui-focusable/src/index.ts(1 hunks)libs/blog/shared/ui-focusable/src/lib/focusable-item.directive.ts(1 hunks)libs/blog/shared/ui-focusable/src/test-setup.ts(1 hunks)libs/blog/shared/ui-focusable/tsconfig.json(1 hunks)libs/blog/shared/ui-focusable/tsconfig.lib.json(1 hunks)libs/blog/shared/ui-focusable/tsconfig.spec.json(1 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icon-item.component.ts(1 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html(1 hunks)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts(1 hunks)libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts(5 hunks)libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html(3 hunks)libs/shared/assets/src/lib/styles/main.scss(3 hunks)tsconfig.base.json(1 hunks)
💤 Files with no reviewable changes (2)
- libs/blog/layouts/ui-layouts/src/lib/footer/partners.ts
- libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.ts
✅ Files skipped from review due to trivial changes (3)
- libs/blog/shared/ui-focusable/README.md
- libs/blog/shared/ui-focusable/tsconfig.lib.json
- libs/blog/shared/ui-focusable/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (9)
- libs/blog/articles/feature-article/src/lib/article-details/article-details.component.html
- libs/blog/layouts/ui-layouts/src/lib/header/components/header-hamburger.component.ts
- libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts
- libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html
- libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html
- libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html
- libs/blog/layouts/ui-navigation/src/lib/language-picker/language-picker.component.ts
- libs/shared/assets/src/lib/styles/main.scss
- libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-24T16:09:56.949Z
Learnt from: DDonochVA
PR: HouseOfAngular/angular-love#390
File: libs/blog/home/feature-home/src/lib/home-page/home-page.component.html:1-26
Timestamp: 2025-02-24T16:09:56.949Z
Learning: In the angular-love project, when multiple UI elements share similar structure and styles (like navigation pills), they should be extracted into reusable components to maintain consistency and reduce duplication.
Applied to files:
libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts
🧬 Code graph analysis (16)
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html (1)
libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.ts (1)
AuthorCardComponent(43-91)
libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.html (2)
libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.ts (2)
onClick(72-76)SearchDialogComponent(56-154)libs/blog/articles/ui-table-of-contents/src/lib/table-of-contents/table-of-contents-scroll-spy.directive.ts (1)
TableOfContentsScrollSpyDirective(20-75)
libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts (2)
libs/blog/articles/feature-article/src/lib/article-share-icons/article-share-icons.component.ts (1)
Component(15-102)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (1)
Component(24-105)
libs/blog/partners/ui-partners/src/lib/partners/partners.component.html (2)
libs/blog/partners/ui-partners/src/lib/partners/partners.component.ts (1)
PartnersComponent(14-17)libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.ts (1)
PartnersListComponent(12-14)
libs/blog/home/feature-home/src/lib/home-page/home-page.component.ts (3)
libs/blog/shell/feature-shell-web/src/lib/blog-shell.routes.ts (1)
HomePageComponent(30-32)libs/blog/partners/ui-partners/src/lib/partners/partners.component.ts (1)
PartnersComponent(14-17)libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.ts (1)
PartnersListComponent(12-14)
libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (5)
libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts (1)
Component(16-51)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
Component(21-47)libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts (1)
Component(13-50)libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (1)
Component(28-135)libs/blog/layouts/ui-layouts/src/lib/header/header-nav-items.ts (1)
headerNavItems(3-25)
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts (2)
libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts (1)
Component(5-15)libs/blog/shared/ui-social-media-icons/src/lib/social-media-icon-item.component.ts (1)
Component(30-73)
libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts (3)
libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (1)
Component(24-105)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
Component(21-47)libs/blog/layouts/ui-layouts/src/lib/header/header-nav-items.ts (1)
headerNavItems(3-25)
libs/blog/home/feature-home/src/lib/home-page/home-page.component.html (1)
libs/blog/articles/feature-list/src/lib/articles-list-container.component.ts (1)
ArticlesListContainerComponent(11-11)
libs/blog/shared/ui-focusable/src/lib/focusable-item.directive.ts (4)
libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-improvements/become-author-improvements.component.ts (1)
BecomeAuthorImprovementsComponent(12-14)libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts (1)
UiDifficultyComponent(94-105)libs/blog/shared/ui-tooltip/src/lib/ui-tooltip/tooltip.directive.ts (1)
TooltipDirective(19-72)libs/blog/ad-banner/ui/src/lib/infinite-slider-directive/al-infinite-slider.directive.ts (1)
AlInfiniteSliderDirective(35-104)
libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (1)
libs/blog/shared/ui-avatar/src/lib/avatar.component.ts (1)
AvatarComponent(13-17)
libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (1)
libs/blog/app-theme/data-access-app-theme/src/app-theme.store.ts (1)
AppThemeStore(11-38)
libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts (1)
libs/blog/articles/data-access/src/lib/state/article-list.store.ts (1)
ArticleListStore(28-65)
libs/blog/layouts/ui-layouts/src/lib/header/components/header-logo.component.ts (2)
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.ts (1)
Component(24-102)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (1)
Component(24-105)
libs/blog/layouts/ui-layouts/src/lib/header/header-nav-items.ts (1)
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
NavItem(13-19)
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.ts (3)
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (2)
Component(21-47)NavItem(13-19)libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-logo.component.ts (1)
Component(4-23)libs/blog/layouts/ui-layouts/src/lib/footer/partners.ts (2)
partnersList(17-36)hoaHireUs(3-8)
🔇 Additional comments (45)
libs/blog/articles/feature-article/src/lib/article-share-icons/article-share-icons.component.ts (2)
32-37: Excellent accessibility improvement.Adding
aria-hidden="true"to decorative icons is correct since the parent anchor already provides semantic context viaaria-label.
48-61: Proper accessibility pattern for state icons.Correctly hides decorative state icons from assistive technology while the button's
aria-labelprovides the semantic context.libs/blog/partners/ui-partners/src/lib/partners/partners.component.html (1)
3-3: Theme clean-up LGTM; verify spacing/contrast.Switch to bg-transparent aligns with the light-mode refactor. Please double-check vertical rhythm (margins were removed) and text/background contrast in both themes.
libs/blog/search/feature-search/src/lib/feature-search/global-search.component.ts (2)
24-29: LGTM!Adding
aria-hidden="true"to the decorative SVG icon is an accessibility best practice, since the parent button already has a descriptivearia-label. This pattern is consistent with usage inarticle-share-icons.component.ts.
7-7: LGTM — translation keynav.open_search_dialogexists in all supported locales.libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.html (2)
7-7: LGTM!Replacing
[ngClass]with[class]is a good refactor. The[class]binding is more direct and equally effective for simple conditional class application.
20-20: LGTM!Adding
aria-hidden="true"to the decorative magnifier icon is correct, as the button likely has other accessible text or context for screen readers.libs/blog/roadmap/ui-roadmap-dialog/src/lib/roadmap-dialog-footer/roadmap-dialog-footer.component.ts (1)
28-28: LGTM! Good accessibility improvement.Marking the decorative SVG icon as
aria-hidden="true"is correct, since the parent anchor already provides an accessible label viaaria-label. This prevents screen readers from announcing redundant information.libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card.component.html (1)
33-33: Good accessibility improvement on decorative icon.Adding aria-hidden="true" to the clock icon is correct.
libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html (2)
10-16: Empty alt is acceptable if the title provides the name.alt="" is fine when the card’s text (title) names the link. If the image conveys unique info not present in text, provide descriptive alt.
33-33: Good: mark decorative clock icon hidden.aria-hidden="true" on the clock is correct.
libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (1)
34-34: Good: decorative icon hidden from AT.aria-hidden="true" is appropriate here.
tsconfig.base.json (1)
228-230: LGTM! Path alias properly configured.The new
@angular-love/ui-focusablealias follows existing conventions and correctly points to the library's public API barrel file.libs/blog/shared/ui-focusable/src/index.ts (1)
1-1: LGTM! Clean public API barrel.Standard re-export pattern for exposing the directive as the library's public API.
libs/blog/shared/ui-focusable/tsconfig.spec.json (1)
1-16: LGTM! Standard Jest test configuration.The test TypeScript configuration properly extends the base config and includes all necessary test files and types for Jest testing.
libs/blog/shared/ui-focusable/project.json (1)
1-20: LGTM! Proper Nx project configuration.The project configuration correctly defines the library with appropriate tags, prefix, and build targets for testing and linting.
libs/blog/shared/ui-focusable/.eslintrc.json (1)
1-37: LGTM! Standard Angular ESLint configuration.The linting rules properly enforce the "al" prefix convention for directives and components, with appropriate selector styles. The
prefer-standalonerule is intentionally disabled, aligning with the project's architecture decisions.libs/blog/shared/ui-focusable/src/test-setup.ts (1)
1-9: LGTM! Proper Jest setup with strict checking.The test setup correctly imports jest-preset-angular and enables strict error checking for unknown elements and properties, which helps catch template errors early during testing.
libs/blog/shared/ui-focusable/jest.config.ts (1)
1-22: LGTM! Comprehensive Jest configuration.The Jest configuration properly sets up transforms for Angular testing, includes the test setup file, and configures coverage output and snapshot serializers appropriately.
libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts (4)
1-2: LGTM! Well-structured imports for keyboard navigation.The imports correctly bring in Angular CDK's FocusKeyManager for accessible keyboard navigation, along with the necessary lifecycle hooks and custom focusable directive. The additions support the new keyboard navigation feature effectively.
Also applies to: 4-4, 10-11, 29-29, 44-44, 51-51
58-58: LGTM! Proper setup for focus management.The component correctly implements AfterViewInit and declares the necessary fields for focus management. The definite assignment assertions are safe since initialization occurs in the ngAfterViewInit lifecycle hook.
Also applies to: 67-70
96-103: LGTM! Correct FocusKeyManager initialization.The ngAfterViewInit implementation properly initializes the FocusKeyManager with horizontal navigation and wrap behavior. Setting the first item as active provides a good initial focus state.
105-107: LGTM! Proper keyboard event delegation.The onPillKeydown method correctly delegates keyboard events to the FocusKeyManager, enabling accessible keyboard navigation through the category pills.
libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html (3)
2-6: LGTM! Proper ARIA tablist implementation.The container correctly implements the tablist role with an accessible label, providing proper semantic structure for the category navigation.
Also applies to: 20-20
35-37: LGTM! Excellent refactoring to reduce duplication.The newsletter card has been properly extracted into a reusable template and injected at appropriate locations in both the data-present and loading states. This reduces code duplication and improves maintainability.
Based on learnings about extracting reusable UI components in the angular-love project.
Also applies to: 41-41, 51-58
46-46: LGTM! Button sizing added.The addition of size="medium" provides explicit size control for the button component.
libs/blog/shell/feature-shell-web/src/lib/root-shell.component.ts (2)
26-26: Import alias looks goodAlias resolves and barrel export exists per prior checks. No issues.
36-39: Header/Footer theme API is compatible
HeaderComponent provides a signal-friendlythemeinput and athemeToggleoutput, and FooterComponent provides a signal-friendlythemeinput—no changes needed.libs/blog/home/feature-home/src/lib/home-page/home-page.component.html (1)
5-5: LGTM – spacing/class additionThe added Tailwind classes make the list block-level with spacing; fits the layout.
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.scss (1)
26-36: Verify Tailwind@screenprocessing in component SCSSEnsure your pipeline runs Tailwind on component SCSS; otherwise
@screenwon’t compile. If unsupported, replace with native media queries.Also applies to: 39-45
apps/blog/src/assets/i18n/en.json (6)
13-13: LGTM: Translation key for article types added.The new
articleTypeskey underhomePageis well-named and consistent with the existing structure. It provides clear user-facing text for article categorization UI.
26-29: LGTM: Navigation keys added for expanded menu structure.The new navigation keys (
guides,news,in_depth,partnership) follow consistent naming conventions and provide clear labels. These support the navigation enhancements visible in the header and footer components.
34-35: LGTM: Accessibility labels for theme toggle and search dialog.The
toggle_themeandopen_search_dialogkeys provide clear, action-oriented labels for accessibility (aria-label). These align with the theme toggle button and search dialog functionality introduced in the header component.
38-39: LGTM: Language picker accessibility label added.The
select_langkey underlanguagePickerprovides a clear accessibility label for the language selection UI, consistent with the existing structure.
42-46: LGTM: Footer section restructured for better organization.The new
footerobject consolidates partner and description content, improving i18n structure. MovingpartnersandmainPartnerfromhomePagetofooteraligns with the footer component's refactored UI.
232-232: LGTM: Social media section title added.The
titlekey undersocialLinksprovides a clear heading for the social media section, improving UI clarity and accessibility in the footer.apps/blog/src/assets/i18n/pl.json (1)
13-13: LGTM: Polish translations maintain parity with English file.All i18n changes in the Polish translation file mirror the structure and keys added to
en.json:
- Article types, navigation keys (guides, news, in_depth, partnership)
- Theme toggle and search dialog accessibility labels
- Language picker label
- Reorganized footer section with partners and description
- Social media section title
The Polish translations are contextually appropriate and maintain structural consistency.
Also applies to: 26-29, 34-35, 38-39, 42-46, 235-235
libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (4)
4-4: LGTM: All imports are properly utilized.The new imports support the theme toggle functionality:
computedforthemeSwitchIconcalculationTranslocoDirectivefor i18n in the templateFastSvgComponentfor icon renderingheaderNavItemsfor centralized navigation configurationAll imports are used and necessary.
Also applies to: 9-10, 22-22
27-29: LGTM: Template changes properly integrate theming and i18n.The template updates are well-implemented:
- Transloco directive correctly scoped to
'nav'for localized labels- Theme toggle button includes proper accessibility via
aria-labelbound tot('toggle_theme')data-testidattribute enables testing- Icon rendering uses the computed
themeSwitchIconsignal- Navigation binding updated to use centralized
navItemsAlso applies to: 36-36, 45-56
87-87: LGTM: Theme management properly implemented with correct UX logic.The component properties correctly implement theme toggling:
navItemsextraction toheaderNavItemsimproves modularitythemeinput is required, ensuring parent provides theme statethemeToggleoutput has correct void signature for event emissionthemeSwitchIconcomputed logic follows proper UX pattern: light theme shows 'moon' icon (indicating user can switch to dark), dark theme shows 'sun' icon (indicating user can switch to light)Also applies to: 90-90, 94-94, 98-100
82-83: LGTM: Component imports properly registered.
FastSvgComponentandTranslocoDirectiveare correctly added to the component's imports array, enabling their use in the template.libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
29-44: LGTM: Component refactored for external configuration and layout flexibility.The navigation component has been properly refactored:
navItemsas required input promotes external configuration and reusability- Removed internal state (
showNewsletter,filteredNavItems, staticnavItems) improves separation of concernslistClassescomputed provides flexible layout handling for vertical, horizontal, and grid layouts- The
whiteFontboolean input provides basic theming support adequate for current requirementslibs/blog/layouts/ui-layouts/src/lib/footer/footer.component.ts (3)
1-22: LGTM: All imports properly utilized for footer enhancements.The new imports support theme-aware rendering, responsive behavior, and enhanced footer navigation:
BreakpointObserverandtoSignalfor responsiveisXlScreensignalcomputedandinputfor theme-based logo selectionNavigationComponent,PartnersComponent,NgOptimizedImage,TranslocoDirectivefor UI componentshoaHireUsandpartnersListfor partner data displayAll imports are necessary and used.
26-33: LGTM: Component imports array properly updated.
PartnersComponent,NgOptimizedImage, andTranslocoDirectiveare correctly registered in the imports array, enabling their use in the footer template.
39-48: LGTM: Theme-aware logo selection and partner data properly exposed.The footer correctly implements theme-based rendering:
themeinput is required, ensuring parent provides theme state (consistent with header)hoaLogoSrccomputed properly switches between dark (HOA_logo_white.png) and light (HOA_logo_blue.png) logo assetspartnersListandhoaHireUsexpose partner data for template binding
...ature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html
Show resolved
Hide resolved
| [ngClass]="{ 'translate-y-[100%]': isOpened() }" | ||
| [attr.aria-hidden]="isOpened()" | ||
| > |
There was a problem hiding this comment.
Invert aria-hidden
Set aria-hidden to true when the menu is hidden, false when visible.
- [attr.aria-hidden]="isOpened()"
+ [attr.aria-hidden]="!isOpened()"📝 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.
| [ngClass]="{ 'translate-y-[100%]': isOpened() }" | |
| [attr.aria-hidden]="isOpened()" | |
| > | |
| [ngClass]="{ 'translate-y-[100%]': isOpened() }" | |
| [attr.aria-hidden]="!isOpened()" | |
| > |
🤖 Prompt for AI Agents
In
libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts
around lines 21 to 23, the aria-hidden attribute is currently set to isOpened()
(true when open) but must be inverted so aria-hidden is true when the menu is
hidden and false when visible; change the binding to return the inverse (e.g.,
use !isOpened() or the string form isOpened() ? 'false' : 'true') so the
attribute reflects visibility correctly.
| class="text-al-foreground p-2 font-medium" | ||
| [class.md:p-6]="!cols() && layout() === 'horizontal'" | ||
| [href]="item.link" |
There was a problem hiding this comment.
Fix href type and add rel for external links
NavItem.link is string[]; href must be string. Also add rel for security.
- class="text-al-foreground p-2 font-medium"
- [class.md:p-6]="!cols() && layout() === 'horizontal'"
- [href]="item.link"
+ class="text-al-foreground p-2 font-medium"
+ [class.md:p-6]="!cols() && layout() === 'horizontal'"
+ [href]="item.link[0]"
+ rel="noopener noreferrer"📝 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.
| class="text-al-foreground p-2 font-medium" | |
| [class.md:p-6]="!cols() && layout() === 'horizontal'" | |
| [href]="item.link" | |
| class="text-al-foreground p-2 font-medium" | |
| [class.md:p-6]="!cols() && layout() === 'horizontal'" | |
| [href]="item.link[0]" | |
| rel="noopener noreferrer" |
🤖 Prompt for AI Agents
In libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html
around lines 25-27, item.link is an array (string[]) but href expects a string
and external links should include a rel for security; convert/derive a single
string for href (e.g. compute a string in the component like href =
Array.isArray(item.link) ? item.link.join('/') : item.link) and bind that string
to [href], and add a rel attribute for external targets (e.g. set rel="noopener
noreferrer" when the href starts with "http" or when target is "_blank") —
implement the string conversion and conditional rel logic in the component class
and update the template to bind the resulting href string and rel value.
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts
Show resolved
Hide resolved
...blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.html
Show resolved
Hide resolved
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html
Show resolved
Hide resolved
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.ts
Show resolved
Hide resolved
|
PR is detected, will deploy to dev environment |
|
Deployed to dev environment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts (1)
15-41: Addstandalone: trueto the component decorator.The component decorator includes an
importsarray (line 40), which is only valid for standalone components. Withoutstandalone: true, Angular will throw a runtime error.Apply this diff to add the missing property:
@Component({ selector: 'al-header-mobile-menu', template: `...`, styleUrl: './header-mobile-menu.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, + standalone: true, imports: [NavigationComponent, LanguagePickerComponent], })
♻️ Duplicate comments (1)
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html (1)
23-29: Still need single-string href +relon external linksExternal links are still bound with
[href]="item.link", butNavItem.linkis astring[], so the browser receives a comma-joined value (a,b) instead of a proper URL. Additionally,target="_blank"withoutrel="noopener noreferrer"leaves us open to reverse-tabnabbing. Please derive a concrete string for the external href (e.g.,item.link[0]or a helper that normalizes the array) and add the protectiverel.- [href]="item.link" - target="_blank" + [href]="item.link[0]" + rel="noopener noreferrer" + target="_blank"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/blog/src/assets/i18n/en.json(4 hunks)apps/blog/src/assets/i18n/pl.json(4 hunks)libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html(2 hunks)libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts(2 hunks)libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html(2 hunks)libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.html(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html (1)
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
NavigationComponent(28-68)
libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.html (2)
libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.ts (1)
UiArticleCardComponent(26-30)libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.ts (1)
NewsletterComponent(39-78)
libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts (3)
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
Component(21-47)libs/blog/layouts/ui-layouts/src/lib/header/header.component.ts (1)
Component(24-105)libs/blog/layouts/ui-layouts/src/lib/header/header-nav-items.ts (1)
headerNavItems(3-25)
🔇 Additional comments (8)
apps/blog/src/assets/i18n/pl.json (5)
13-20: Translation aligns with new IA label.Line 13 reads naturally and mirrors the new article type selector copy.
26-36: Nav additions read naturally.The new navigation labels (Lines 26-35) and aria helper text fit Polish expectations and keep the UI consistent.
39-40: Picker prompt is clear.Line 40’s “Wybierz język” is idiomatic and gives the right instruction.
220-223: Search dialog title looks good.Line 220 captures the dialog context precisely.
236-241: Social section heading is accurate.Line 236’s “Media społecznościowe” matches the linked button texts.
libs/blog/layouts/ui-layouts/src/lib/header/components/header-mobile-menu.component.ts (3)
1-14: LGTM!The imports are clean and all imported symbols are used appropriately in the component.
18-36: Excellent accessibility improvement withinert.The template changes look correct. The
[inert]="!isOpened()"binding (line 20) is superior to the previously flaggedaria-hiddenapproach because:
- It makes the entire menu non-interactive when closed
- It hides content from screen readers
- It prevents focus from entering the hidden menu
The animation logic with
[class.translate-y-[100%]]="isOpened()"(line 21) correctly slides the menu into view, and the navigation/language picker bindings properly handle events.
42-50: LGTM!The
navItemsproperty (line 43) is correctly defined and follows the same pattern as the parentHeaderComponent. The readonly modifier is appropriate since the nav items are static.
| "footer": { | ||
| "partners": "Community partnerzy", | ||
| "mainPartner": "Główny partner", | ||
| "al-description": "To jest miejsce, w którym możesz nauczyć się Angulara, odkrywać najlepsze praktyki i być na bieżąco z najnowszymi wiadomościami i trendami" | ||
| }, |
There was a problem hiding this comment.
Polish wording: replace “Community partnerzy”.
Line 43 mixes English and Polish. Please change it to a fully Polish form such as “Partnerzy społeczności”, so the footer copy stays consistent.
🤖 Prompt for AI Agents
In apps/blog/src/assets/i18n/pl.json around lines 42 to 46, the value for the
"partners" key on line 43 mixes English and Polish; replace "Community
partnerzy" with the fully Polish phrasing Partnerzy społeczności so the footer
text is consistently Polish.
| class="bg-al-card relative z-50 m-auto w-full max-w-screen-xl rounded-tl-lg rounded-tr-lg" | ||
| [ngClass]="adBannerStoreVisible() ? 'top-[168px]' : 'top-[84px]'" | ||
| [class]="adBannerStoreVisible() ? 'top-[168px]' : 'top-[84px]'" | ||
| > |
There was a problem hiding this comment.
Restore base classes for the dialog container.
Switching from [ngClass] to [class] replaces the entire class attribute, so all the static classes on Line 9 (layout, spacing, theming) are dropped when the binding runs. That breaks the modal styling/positioning at runtime. Please revert to [ngClass] (and re-import NgClass if needed) or use a class-specific binding so the base classes remain intact.
- <div
- class="bg-al-card relative z-50 m-auto w-full max-w-screen-xl rounded-tl-lg rounded-tr-lg"
- [class]="adBannerStoreVisible() ? 'top-[168px]' : 'top-[84px]'"
- >
+ <div
+ class="bg-al-card relative z-50 m-auto w-full max-w-screen-xl rounded-tl-lg rounded-tr-lg"
+ [ngClass]="adBannerStoreVisible() ? 'top-[168px]' : 'top-[84px]'"
+ >📝 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.
| class="bg-al-card relative z-50 m-auto w-full max-w-screen-xl rounded-tl-lg rounded-tr-lg" | |
| [ngClass]="adBannerStoreVisible() ? 'top-[168px]' : 'top-[84px]'" | |
| [class]="adBannerStoreVisible() ? 'top-[168px]' : 'top-[84px]'" | |
| > | |
| <div | |
| class="bg-al-card relative z-50 m-auto w-full max-w-screen-xl rounded-tl-lg rounded-tr-lg" | |
| [ngClass]="adBannerStoreVisible() ? 'top-[168px]' : 'top-[84px]'" | |
| > |
🤖 Prompt for AI Agents
In
libs/blog/search/feature-search/src/lib/feature-search/search-dialog/search-dialog.component.html
around lines 9 to 11, the template uses [class] which overwrites the entire
class attribute and removes the static classes (layout/spacing/theming),
breaking modal styling; change the binding back to [ngClass] (or use a
class-specific binding like [class.top-...]) so the static class string remains,
and if you use [ngClass] ensure NgClass is imported in the component module or
standalone component imports.
4 participants
1. Add lightmode
Added light theme support with custom color palette. Implemented theme toggle functionality allowing users to switch between dark and light modes. Updated CSS custom properties for proper color contrast in both themes, including background, foreground, borders, and footer colors.
2. Footer and navigation updated
Restructured footer layout with improved grid system for responsive design across breakpoints. Enhanced navigation component with better styling and layout options. Added partner logos display with proper centering and spacing. Improved mobile menu presentation and desktop navigation alignment.
3. A11y improvements
Enhanced accessibility throughout the application:
Summary by CodeRabbit