feat: enable light mode #255
Conversation
Deploying angular-love-client with
|
| Latest commit: |
dee6e92
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0ee79d11.angular-love-client.pages.dev |
| Branch Preview URL: | https://feat-enable-light-mode.angular-love-client.pages.dev |
DDonochVA
force-pushed
the
feat/enable-light-mode
branch
2 times, most recently
from
9c15b33 to
6a5dbae
Compare
|
do not merge until we get a final decision |
DDonochVA
force-pushed
the
feat/enable-light-mode
branch
from
6a5dbae to
5ca24cf
Compare
dokolyski
force-pushed
the
feat/enable-light-mode
branch
from
5ca24cf to
a782d05
Compare
dokolyski
requested review from
DamianBrzezinskiHoA,
mateuszbasinski and
majahendzel-va
|
Some components in the light mode design didn't match the light color palette. The solution is based on light: dark: tailwind modificators and uses custom styles instead of palette colors |
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
Outdated
Show resolved
Hide resolved
DamianBrzezinskiHoA
force-pushed
the
feat/enable-light-mode
branch
2 times, most recently
from
61a7ec4 to
5773b00
Compare
dokolyski
force-pushed
the
feat/enable-light-mode
branch
from
8bc32d7 to
bddb5d1
Compare
Implements KAP-237 KAP-235
+ fix author-card light mode + fix article-card storybook
use AppThemeStore for managing theme and altering styling
dokolyski
force-pushed
the
feat/enable-light-mode
branch
from
bddb5d1 to
f23d6c9
Compare
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html
Outdated
Show resolved
Hide resolved
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts
Outdated
Show resolved
Hide resolved
DamianBrzezinskiHoA
force-pushed
the
feat/enable-light-mode
branch
from
7ef2b4a to
e0949a5
Compare
WalkthroughThis update introduces comprehensive visual and theming enhancements across the project. It adds support for light and dark color schemes using new CSS variables, updates Tailwind and component styles, and modifies several Angular components to utilize dynamic color classes and improved image handling. New dependencies and configuration for theme variants are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant AngularApp
participant TailwindCSS
participant ThemeVariantsPlugin
User->>Browser: Loads site
Browser->>TailwindCSS: Applies styles
TailwindCSS->>ThemeVariantsPlugin: Detects color scheme (light/dark)
ThemeVariantsPlugin-->>TailwindCSS: Applies appropriate theme variables
AngularApp->>Browser: Renders components with dynamic classes
Browser->>User: Displays themed UI with updated styling and images
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
35-35: Consider renaming thewhiteFontproperty to better reflect its purpose.The name
whiteFontis implementation-specific rather than intent-specific. Consider using a more semantic name such ascontrastText,inverseForeground, oralternateTextColorto better express the property's purpose rather than the specific color.As noted in a previous review, naming the property by its meaning rather than the specific color implementation would make the code more maintainable as themes evolve.
🧹 Nitpick comments (13)
libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card-skeleton.component.ts (1)
1-54: Consider adding skeleton loader theme adjustments for light mode.The skeleton loader uses default styling, but might benefit from theme-specific adjustments for better visual harmony in light mode. Consider using CSS variables or Tailwind's theme variants to adjust the skeleton appearance based on the current theme.
<ngx-skeleton-loader class="block h-8" [appearance]="'circle'" [theme]="{ height: '32px', width: '32px', margin: '0', + backgroundColor: 'var(--skeleton-color)', }" />You could define
--skeleton-colorin your theme CSS with appropriate values for light and dark modes.libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts (1)
11-13: Consider using utility classes or design tokens instead of explicit hex.Using
text-[#fff]works but may reduce readability and consistency. Prefer the built-intext-whiteclass or a design token liketext-al-foregroundfor clarity and maintainability:- class="mb-4 hidden text-sm font-bold text-[#fff] lg:block" + class="mb-4 hidden text-sm font-bold text-white lg:block"libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-logo.component.ts (1)
19-21: Ensure contrast consistency in dark mode.The explicit
text-[#bec4ca]color will apply in all modes, which may reduce contrast on dark backgrounds. Consider using a theme variant or design token that adapts per mode, for example:- <small class="hidden pt-1 text-xs text-[#bec4ca] lg:block"> + <small class="hidden pt-1 text-xs light:text-[#bec4ca] dark:text-al-muted lg:block">This approach ensures the muted text is optimized for both light and dark themes.
libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html (1)
3-3: Specify border color when enablinglight:border.Using
light:borderadds the border style but relies on the default color. To ensure consistent theming, explicitly set the border color for light mode:- class="light:border group relative flex h-full w-full flex-row rounded-lg shadow-none" + class="light:border light:border-al-border group relative flex h-full w-full flex-row rounded-lg shadow-none"This ties the border color to your design token under light mode.
libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html (1)
4-7: Prefer semantic utility over raw hex color
Whiletext-[#fff]enforces a white icon color, consider using a semantic utility class liketext-whiteor a design-token class (e.g.,text-al-foreground) to maintain consistency and readability.libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html (2)
4-4: Remove trailing comma in ngClass object
Although JavaScript allows trailing commas, removing it avoids potential linting warnings in Angular template expressions and keeps the object literal clean.
15-15: Consider attribute ordering for readability
Swappingclassbeforesizeimproves consistency with other components. While not functionally required, you may wish to standardize attribute order (static first, then inputs).libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html (1)
24-26: DRY up repeated class and style bindings
Theclass="p-2 font-medium md:p-6"and theme-aware[ngClass]logic is duplicated for both internal and external links. Consider extracting this into a shared directive or consolidating into a single[ngClass]expression to reduce duplication and improve maintainability.Also applies to: 37-39
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html (2)
1-1: Consider using theme variables instead of hardcoded colors for better theme flexibilityYou've changed the footer background from
bg-al-cardto a hardcoded hex colorbg-[#191a22]. While this ensures the footer remains dark in both themes, it creates a maintenance challenge if the design system colors change in the future.Consider using Tailwind's dark mode variant to achieve the same effect but with better maintainability:
-<footer class="w-full bg-[#191a22]"> +<footer class="w-full bg-white dark:bg-[#191a22]">Or even better, use your theme variables:
-<footer class="w-full bg-[#191a22]"> +<footer class="w-full dark:bg-al-card bg-al-grey">
13-13: Consider using theme variables for text colorSimilar to the background color, you've changed the copyright text color from
text-al-mutedto a hardcoded hex colortext-[#6a798b].For better maintainability, consider using the theme variables:
-<small class="copyright text-center text-xs text-[#6a798b] lg:hidden"> +<small class="copyright text-center text-xs text-al-muted lg:hidden">libs/shared/assets/src/lib/styles/main.scss (1)
19-30: Consider adding a class-based theme toggle for more controlWhile using the
prefers-color-schememedia query is excellent for respecting user preferences, many applications also offer a manual toggle to override the system preference.Consider implementing a class-based theme toggle in addition to the media query:
/* In addition to the media query */ :root.light-theme { --primary: 213 1 89; --foreground: 20 21 27; /* rest of the light theme variables */ }Then you can add/remove the
light-themeclass via JavaScript when users toggle their preference.libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (1)
3-3: Consider using a theme-aware background colorYou're using
bg-blackwhich will remain black in both light and dark modes. If this is an article card, it might be better to adapt to the current theme.If the card should adapt to the theme:
-class="relative h-full overflow-hidden rounded-lg bg-black bg-cover bg-no-repeat transition-transform hover:scale-105 motion-reduce:transition-none motion-reduce:hover:scale-100" +class="relative h-full overflow-hidden rounded-lg bg-al-background bg-cover bg-no-repeat transition-transform hover:scale-105 motion-reduce:transition-none motion-reduce:hover:scale-100"If the card should remain dark regardless of theme:
-class="relative h-full overflow-hidden rounded-lg bg-black bg-cover bg-no-repeat transition-transform hover:scale-105 motion-reduce:transition-none motion-reduce:hover:scale-100" +class="relative h-full overflow-hidden rounded-lg dark:bg-black bg-gray-900 bg-cover bg-no-repeat transition-transform hover:scale-105 motion-reduce:transition-none motion-reduce:hover:scale-100"libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (1)
48-48: Consider making excerpt text theme-awareWhile the article title uses theme-aware coloring (
text-al-primary-foreground), the excerpt text uses a fixed color (text-al-pink). Consider making this theme-aware as well to ensure proper contrast in both light and dark modes.- <p class="*:text-al-pink line-clamp-2 *:font-medium *:not-italic"> + <p class="*:text-al-primary-foreground line-clamp-2 *:font-medium *:not-italic">
📜 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 (29)
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-advertisement/become-author-advertisement.component.ts(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(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-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(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-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 (40)
package.json (1)
67-67: Verify tailwindcss-theme-variants beta version compatibility and stability.The new dependency
tailwindcss-theme-variantsis added at version2.0.0-beta.0, which is a beta release. While this plugin is essential for enabling the light/dark mode implementation, using beta versions in production can introduce stability risks.#!/bin/bash # Check for the latest version of tailwindcss-theme-variants and any related issues npm view tailwindcss-theme-variants versions npm view tailwindcss-theme-variants repository # Check if there are more recent versions or tags available echo "Checking GitHub repository for more recent releases..." REPO_URL=$(npm view tailwindcss-theme-variants repository.url 2>/dev/null | sed 's/git+//;s/.git$//') if [ -n "$REPO_URL" ]; then gh api repos/${REPO_URL#https://github.com/}/releases --jq '.[0:3] | .[] | {tag_name, published_at, prerelease}' filibs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html (2)
14-14: Text color explicitly set for better contrast.Adding the
text-whiteclass ensures proper contrast against the pink background, which is important for accessibility in both light and dark modes.
49-49: Consistent text styling for the action button.The
text-whiteclass ensures proper visibility of the button text against the pink background, maintaining consistent styling across the light and dark themes.libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts (1)
1-36:✅ Verification successful
Verify that all component consumers are updated to use the new
whiteFontproperty.Since this component introduces a new input property, ensure that all instances of
<al-navigation>in the codebase properly set this property, particularly in components that were previously relying on default text coloring.
🏁 Script executed:
#!/bin/bash # Find all usages of the al-navigation component in the codebase echo "Checking for usages of al-navigation in the codebase:" rg -A 2 -B 2 "<al-navigation" --type html echo "Checking specifically for instances with whiteFont property:" rg -A 2 -B 2 "\[whiteFont\]" --type htmlLength of output: 1448
No action needed:
whiteFontis set on all<al-navigation>consumers
- Search for
<al-navigation>and[whiteFont]returned a single instance inlibs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html, which already uses[whiteFont]="true".
All consumers are up-to-date.libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card-skeleton.component.ts (1)
10-10: Background made transparent for theme compatibility.Setting the card background to transparent allows it to adapt to the surrounding container's background color, which is essential for proper light/dark mode implementation. This change enables the component to respect the parent container's theme context.
libs/blog/shared/ui-card/src/lib/card.stories.ts (1)
61-65: Transparent background for card story is appropriate.Adding
bg-transparentensures the card renders without its default background, matching other stories and highlighting content correctly under both light and dark themes.libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html (1)
7-9: Great: Applied theme-aware design token for heading color.Using
text-al-primary-foregroundensures the newsletter title adapts correctly across light and dark modes, leveraging your Tailwind theme configuration.libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html (1)
6-6: Correctly applied light mode utilities
Thelight:borderandlight:rounded-lgclasses are added appropriately to enable light-theme borders and corners without impacting the existing dark-mode styling.libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html (1)
45-45: Standardize author name color with theme token
Addingtext-al-foregroundensures the author’s name uses the designated foreground color token in both light and dark modes, aligning with other components.libs/blog/articles/ui-article-list-title/src/lib/ui-article-list-title/ui-article-list-title.component.html (1)
4-4: Align heading color with primary foreground token
Thetext-al-primary-foregroundclass correctly applies the theme’s primary text color, ensuring visual consistency for titles across light and dark modes.libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html (1)
11-11: Ensure badge text remains legible
Thetext-whiteclass is necessary here to guarantee contrast against the pink background, so this addition is appropriate.libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.html (1)
15-18: Ensure image priority consistency across card variants
Nice catch adding[imagePriority]="imagePriority()"to the compact card. This brings the compact variant in line with the regular and horizontal cards and ensures all images respect the same loading priority.libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-advertisement/become-author-advertisement.component.ts (1)
2-2: Add TranslocoDirective to standalone component imports
ImportingTranslocoDirectiveand adding it to theimportsarray enables template-level translation support. Ensure that@jsverse/translocois listed in your dependencies and that this directive is compatible with standalone components.Also applies to: 13-13
libs/blog/become-author/feature-become-author-page/src/lib/become-author-page/become-author-page.component.html (2)
34-36: Consistent button text color on call-to-action
Great addition oftext-whitefor the button text: it ensures clear contrast against the pink background.
64-66: Maintain uniform styling for secondary action buttons
Addingtext-center text-sm font-bold text-whitealigns the article rules button with the primary CTA styling—this maintains visual consistency.libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html (3)
2-4: Apply primary-foreground token to page title
Addingtext-al-primary-foregroundensures the title adapts to the new theming tokens—this matches other modules and improves consistency.
6-7: Use transparent background for card container
bg-transparentallows the card to inherit the parent’s background, which is necessary for theme-driven backgrounds. Verify it renders correctly in both light and dark contexts.
38-40: Apply primary-foreground token to authors section title
Consistent with the main title, addingtext-al-primary-foregroundhere aligns the authors title with the updated design language.libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html (1)
6-6: Good addition of conditional text coloringAdding the
[whiteFont]="true"input to the navigation component properly supports the theming approach by allowing conditional text coloring in the navigation.libs/shared/assets/src/lib/styles/main.scss (2)
11-11: Good addition of new color variables to the theme systemThe addition of
--primary-foregroundand--greyvariables to the default theme is appropriate and follows the established pattern.Also applies to: 16-16
19-30: Well-implemented light theme using media queriesThe implementation of the light theme using CSS variables and the
prefers-color-scheme: lightmedia query is well done. This allows the application to automatically switch between dark and light themes based on the user's system preferences.libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (1)
6-13: Good use of NgOptimizedImage for performanceUsing
NgOptimizedImagewith thepriorityattribute is a good practice for optimizing image loading performance, especially for above-the-fold content.libs/blog/shared/ui-card/src/lib/card.component.ts (3)
72-72: Good implementation of conditional themingThe updated
GradientCardDirectivenow properly uses the dark mode variant (dark:bg-al-background) with the gradient background, allowing it to adapt to the current theme.
100-100: Good addition of theme-aware background to the card componentAdding the
bg-al-cardclass to the card component correctly integrates it with the theming system, allowing cards to adapt their background color to the current theme.
100-100:❓ Verification inconclusive
Verify accessibility contrast with the new background
With the addition of the
bg-al-cardbackground, ensure that text elements within cards maintain sufficient contrast in both light and dark modes.
🌐 Web query:
What is the recommended contrast ratio for text accessibility according to WCAG standards?💡 Result:
Recommended Contrast Ratio for Text Accessibility (WCAG Standards)
WCAG (Web Content Accessibility Guidelines) standards set specific minimum contrast ratios to ensure text is readable for users with visual impairments. The requirements vary by compliance level and text size:
Level AA (Minimum Accessibility)
Normal text: A contrast ratio of at least 4.5:1 between the text and its background is required[1][2][3][5][9][10].
Large text: The minimum ratio lowers to 3:1 for large-scale text because it is generally easier to perceive[1][2][3][5][9][10].
- Large text is defined as 18pt (24px) regular or 14pt (19px) bold or larger[6][9][10].
Level AAA (Enhanced Accessibility)
- Normal text: A higher contrast ratio of at least 7:1 is required[5][6][9][10].
- Large text: The minimum ratio is 4.5:1[5][6][9][10].
Special Cases
- Logotypes (logos, brand names): No contrast requirement[1][2][3].
- Incidental or decorative text: No contrast requirement[1][2][3].
Summary Table
Compliance Level Normal Text Large Text WCAG AA 4.5:1 3:1 WCAG AAA 7:1 4.5:1 In practice: To meet most organizational and legal requirements, content creators should ensure normal text meets at least a 4.5:1 contrast ratio and large text meets at least a 3:1 ratio[3][5][9][10].
References to the guidelines can be found in the official W3C documentation and accessibility checkers such as WebAIM, Accessible Web, and institutional brand guidelines.
Citations:
- 1: https://www.w3.org/WAI/WCAG21/Understanding/contrast-minimum
- 2: https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-contrast.html
- 3: https://webaim.org/articles/contrast/
- 4: https://dequeuniversity.com/rules/axe/4.10/color-contrast
- 5: https://help.siteimprove.com/support/solutions/articles/80001050538-accessibility-rule-color-contrast-does-not-meet-requirement-explained
- 6: https://webaim.org/resources/contrastchecker/
- 7: https://www.oregon.gov/ode/accessibility/checklist/pages/contrast.aspx
- 8: https://my.willamette.edu/site/digital-accessibility/guidelines/color-contrast
- 9: https://accessibleweb.com/color-contrast-checker/
- 10: https://brand.ucla.edu/fundamentals/accessibility/color-type
Verify card text contrast meets WCAG AA standards
With the new
bg-al-cardbackground, ensure all text inside the card maintains at least:
- 4.5:1 contrast ratio for normal-sized text
- 3:1 contrast ratio for large text (≥18 pt/24 px or ≥14 pt/19 px bold)
Check both light and dark mode variants using a contrast checker (e.g., WebAIM Contrast Checker) to confirm compliance.
libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (4)
3-3: Good implementation of theme-aware bordersThe border styling has been properly updated to be visible in light mode while remaining transparent in dark mode, which aligns with the light mode feature implementation.
21-25: Nice touch adding image priority to the avatarThe addition of the
[priority]="imagePriority()"binding improves performance by properly prioritizing the loading of avatar images.
26-26: Well-implemented foreground text stylingUsing
text-al-primary-foregroundensures the author name will have proper contrast in both light and dark modes.
41-45: Properly implemented heading text stylingThe heading now has appropriate theme-aware text color and consistent styling with
*:text-al-primary-foregroundand*:not-italic.libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (1)
28-38: Good conversion to al-card with proper directivesThe nested card is now properly implemented using the
<al-card>component with appropriate directives, which will ensure consistent styling across the application.libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts (4)
28-30: Well-implemented theme-aware background for left blockThe background styling has been properly updated to support both light and dark modes, using transparent backgrounds in light mode and proper background colors in dark mode.
47-50: Good update to shadow styling for dot containersThe shadow styling has been properly updated to be transparent in light mode and use the appropriate shadow color in dark mode, creating a consistent visual appearance across themes.
80-82: Well-implemented theme-aware background for right blockThe background styling has been properly updated to support both light and dark modes, matching the left block's implementation for consistency.
19-22: Verify difficulty indicator contrast in light modePlease verify that the difficulty indicator maintains sufficient contrast in light mode, especially for the text and dots. The container now uses a transparent background in light mode which might affect readability depending on what's beneath it.
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (2)
1-1: Good conversion to standalone component with image optimizationThe component has been properly converted to a standalone component with all necessary imports, including
NgOptimizedImagefor better image loading performance.Also applies to: 13-20
25-25:✅ Verification successful
Good addition of imagePriority input
The new
imagePriorityinput allows for controlling image loading priority, which is important for performance optimization. This matches the implementation in the regular card component.Make sure this property is properly used in the HTML template (not shown in this review) to prioritize the loading of featured images.
🏁 Script executed:
#!/bin/bash # Check if the imagePriority input is used in the HTML template cat libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html | grep -A 5 -B 5 "priority"Length of output: 493
Confirmed:
imagePriorityinput is implemented and used correctlyThe
imagePriorityinput is added in the component and is referenced in the template as[priority]="imagePriority()", mirroring the regular card component’s behavior. No further changes needed.tailwind.preset.js (4)
1-5: Good implementation of system-preference based theming.The integration of
tailwindcss-theme-variantsplugin is a good approach for enabling system-driven light and dark themes. Using media queries instead of class toggling is more user-friendly as it automatically adapts to user preferences.
13-14: New color tokens follow consistent naming patterns.The additions of
al-primary-foregroundandal-greyfollow the existing naming conventions and properly leverage CSS variables with alpha value support, maintaining consistency with the rest of the color tokens.Also applies to: 22-22
39-51: Good plugin configuration for theme variants.The plugin configuration correctly sets up light and dark themes using the appropriate media queries. This enables automatic theme switching based on system preferences, which improves user experience.
Note that this approach requires corresponding CSS variables to be defined for both themes in your stylesheet, which according to the summary are added in
libs/shared/assets/src/lib/styles/main.scss.
35-35:❓ Verification inconclusive
Verify the visual impact of changing box shadow variable.
The change from
--borderto--greyfor theal-full-borderbox shadow might have visual implications. Ensure this change aligns with the design specifications for both light and dark modes.
🏁 Script executed:
#!/bin/bash # Check where the al-full-border shadow is being used in the codebase rg "al-full-border" --type html --type css --type scssLength of output: 83
🏁 Script executed:
#!/bin/bash # Locate the definition in the Tailwind preset rg "al-full-border" -n tailwind.preset.js # Search for any usage of the corresponding utility class across markup and scripts rg "shadow-al-full-border" -n --glob "*.{html,js,ts,jsx,tsx,vue,css,scss}"Length of output: 318
Verify visual impact of
al-full-borderchangeThe
al-full-bordershadow is now defined as:// tailwind.preset.js:35 'al-full-border': '0px 0px 0px 999px rgba(var(--grey) / 1)',and is only applied here:
- libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts:49 (
'shadow-al-full-border': !isColorBackground())Please review this change in both light and dark modes against your design specs to ensure the switch from
--borderto--greystill meets the intended look and accessibility requirements.
| width="1215" | ||
| height="750" | ||
| /> | ||
| <div class="relative flex h-full flex-col justify-between text-[#fff]"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use theme variables for text color instead of hardcoded white
You're using a hardcoded white color (text-[#fff]) which won't adapt to different themes.
For proper theme support, use the foreground color variable:
-<div class="relative flex h-full flex-col justify-between text-[#fff]">
+<div class="relative flex h-full flex-col justify-between text-al-foreground">If the text should remain white regardless of theme, use the Tailwind class:
-<div class="relative flex h-full flex-col justify-between text-[#fff]">
+<div class="relative flex h-full flex-col justify-between text-white">
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts
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 |
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores