-
Notifications
You must be signed in to change notification settings - Fork 8
Revert "feat: enable light mode (#255)" #443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This reverts commit 600f623.
WalkthroughThis set of changes removes light theme support and related CSS variables, eliminates theme-variant-based Tailwind configuration, and standardizes styling across components by removing light/dark mode-specific and foreground/background color classes. Several Angular components are updated to simplify or remove color and background class bindings, and some input properties are refactored or removed for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant FooterLogoComponent
App->>FooterLogoComponent: Passes required currentYear input
FooterLogoComponent->>FooterLogoComponent: Renders copyright with currentYear
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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 (
|
PR is detected, will deploy to dev environment |
Deployed to dev environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
libs/blog/articles/feature-latest-articles/src/lib/feature-latest-articles/feature-latest-articles.component.ts (1)
33-46
: 🛠️ Refactor suggestionRemove duplicate
CardComponent
entry in imports
Theimports
array listsCardComponent
twice (lines 36 and 41). This redundancy should be cleaned up to avoid confusion.@Component({ imports: [ UiArticleCardComponent, NewsletterComponent, - CardComponent, GradientCardDirective, NgClass, TranslocoDirective, ArticleRegularCardSkeletonComponent, - CardComponent, RepeatDirective, RouterLink, ButtonComponent, PillDirective, ], ... })
🧹 Nitpick comments (1)
libs/blog/articles/ui-article-card/src/lib/components/article-horizontal-card/article-horizontal-card.component.html (1)
3-3
: Verify content overflow with the new max-height
Themax-h-52
utility caps the card height at 13rem; ensure long titles or excerpts aren’t clipped unexpectedly. Consider addingoverflow-hidden
or usingline-clamp
on inner text elements to gracefully handle overflow.
📜 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.svg
is excluded by!**/*.svg
pnpm-lock.yaml
is 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
(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
(1 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
(1 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
(0 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
(0 hunks)package.json
(0 hunks)tailwind.preset.js
(2 hunks)
💤 Files with no reviewable changes (3)
- package.json
- libs/shared/assets/src/lib/styles/main.scss
- libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.ts
🔇 Additional comments (50)
libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html (2)
14-14
:❓ Verification inconclusive
Verify text contrast after color class removal
The
text-white
class has been removed from this paragraph element, which is on a pink background (bg-al-pink
). Ensure there's still sufficient color contrast for accessibility.
🏁 Script executed:
#!/bin/bash # Check if there's a global text color applied in CSS for elements within bg-al-pink rg -A 5 "bg-al-pink" --type css --type scssLength of output: 72
🏁 Script executed:
#!/usr/bin/env bash # Locate the Tailwind config file config=$(rg --files | rg -E "tailwind\\.config\\.(js|ts|cjs)" | head -n1) echo "Found Tailwind config at: $config" # Extract the custom 'al-pink' color definition (and surrounding lines for context) rg -n "al-pink" -C3 "$config"Length of output: 600
Unable to locate
bg-al-pink
definition—please verify contrast manuallyI couldn’t find any Tailwind config or CSS/SCSS entries for
bg-al-pink
. Withtext-white
removed, please ensure the default text color still meets WCAG contrast ratios against this pink background.• File: libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html
– Line 14:<p class="text-base font-bold">{{ index() + 1 }}</p>
49-49
:❓ Verification inconclusive
Verify text contrast on pink button
The
text-white
class has been removed from this button with a pink background (bg-al-pink
). Please check that the text remains readable with sufficient contrast.
🏁 Script executed:
#!/bin/bash # Verify if there's a default text color for buttons with bg-al-pink rg -A 5 "bg-al-pink" --type css --type scss # Check global button styling rg "button.*{" --type css --type scssLength of output: 194
🏁 Script executed:
#!/bin/bash set -e # Locate any Tailwind config files fd 'tailwind.config.*' --hidden || true # Inspect custom color definitions in Tailwind config(s) fd 'tailwind.config.*' --hidden -x rg "al-pink" -n {} # Find all occurrences of bg-al-pink in the repo rg "bg-al-pink" -n . # Show surrounding lines to see if a default text color is applied nearby rg -C2 "bg-al-pink" -n .Length of output: 220
Ensure text contrast on pink button
The
text-white
utility was removed from this button with thebg-al-pink
background. Without an explicit text-color class, the default color may not meet WCAG contrast requirements against your custom pink. Please verify in-browser (or using a contrast checker) that the resulting text color onbg-al-pink
achieves at least a 4.5:1 contrast ratio, and if not, add an appropriatetext-…
class (for example,text-white
) to restore sufficient readability.• File:
libs/blog/writing-rules/feature-writing-rules/src/lib/rules-row/rules-row.component.html
Line: 49libs/blog/partners/ui-partners/src/lib/partners-list/partners-list.component.html (1)
6-6
: Light theme border styling removedThe
light:border
andlight:rounded-lg
classes have been removed, which aligns with disabling light mode support. The element still maintains rounded corners through therounded-lg
class, but be aware that partner logos no longer have a border in light mode.libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-social-media-icons.component.ts (1)
12-12
:❓ Verification inconclusive
Confirm footer heading readability
The
text-[#fff]
class has been removed from this heading. If the footer background is dark, verify that the text is still readable with the default text color.
🏁 Script executed:
#!/bin/bash # Check footer component's background color rg -A 10 -B 5 "selector: 'al-footer'" --type typescript # Check if there's a global text color for headings in the footer rg "footer.*h3" --type css --type scssLength of output: 168
Confirm footer heading readability
Please verify that on your dark‐themed footer the heading still meets contrast requirements now that the
text-[#fff]
class has been removed. If the default text color renders too dark on the footer background, add a Tailwind class such astext-white
(or another light shade) back to the heading.Check:
- Footer SCSS/CSS files under
libs/blog/layouts/ui-layouts/src/lib/footer/
for anybackground
orcolor
rules.- Global typography or theme settings that might override default text color in this component.
libs/blog/shared/ui-pill/src/lib/pill.directive.ts (1)
12-12
:❓ Verification inconclusive
Pill flat variant text contrast needs verification
The
text-white
class has been removed from the flat variant styling. Since the background is semi-transparent primary color (bg-al-primary/90
), verify that text remains readable with the default text color.
🏁 Script executed:
#!/bin/bash # Check how this pill is used in the application rg "alPill.*variant.*flat" --type html --type typescript # Check if there's a global text color that would apply rg "al-primary" --type css --type scssLength of output: 161
Pill flat variant text contrast needs manual verification
- In
libs/blog/shared/ui-pill/src/lib/pill.directive.ts
(line 12), thetext-white
class was removed from theflat
variant, leaving onlybg-al-primary/90
.- A semi-transparent primary background can reduce contrast with the default text color (Tailwind’s base text color).
- Please verify in your running app (or via a WCAG 2.1 AA contrast checker) that text over
bg-al-primary/90
remains easily readable.- If contrast falls below acceptable levels, re-introduce a high-contrast text class (e.g.
text-white
) or explicitly set a suitable text color.libs/blog/articles/ui-article-list-title/src/lib/ui-article-list-title/ui-article-list-title.component.html (1)
4-4
: Ensure heading uses the correct default color
Removingtext-al-primary-foreground
relies on the component inheriting the appropriate default text color. Please verify that the default theme provides the intended contrast and readability across all breakpoints.libs/blog/shared/ui-card/src/lib/card.stories.ts (1)
61-66
: Validate default background behavior in Storybook
The explicitbg-transparent
class was removed from<al-card>
. Ensure that the card’s default background styling (fromCardComponent
) matches design expectations in all story variants (regular, withImage, highlighted).libs/blog/shared/ui-social-media-icons/src/lib/social-media-icons.component.html (1)
4-4
: Confirm icon color inheritance
Dropping the explicittext-[#fff]
class means color will now come from the parent or default component styles. Please verify that the icons render with the correct color in the footer and other contexts without visual regressions.libs/blog/articles/ui-article-card/src/lib/components/article-hero-card/article-hero-card-skeleton.component.ts (1)
10-11
: Consistent card background simplification applied
Removing thebg-transparent
class aligns with the PR’s unified card styling; this skeleton will now inherit the default card background. No functional impact observed.libs/blog/about-us/feature-about-us/src/lib/feature-about-us/feature-about-us.component.html (3)
2-4
: Removed explicit foreground color on main heading
Droppingtext-al-primary-foreground
in favor of default text color simplifies theming. The heading remains semantically correct and visually bold.
6-12
: Card background reset to default
Omittingbg-transparent
on<al-card>
standardizes the card’s appearance with other components. This change is in line with the global style update.
38-40
: Removed explicit foreground color on authors subheading
Consistency maintained by letting the default color apply—removal oftext-al-primary-foreground
is appropriate here.libs/blog/authors/ui-author-card/src/lib/author-card/author-card.component.html (1)
45-49
: Dropped explicit text color on author name
Removingtext-al-foreground
streamlines the styling; the default color will apply consistently across themes.libs/blog/newsletter/feature-newsletter/src/lib/feature-newsletter/newsletter.component.html (1)
8-9
: Simplified newsletter title styling
Removing thetext-al-primary-foreground
class aligns this title with the updated global text styling approach. No accessibility or layout regressions noted.libs/blog/articles/ui-article-card/src/lib/ui-article-card/ui-article-card.component.html (1)
15-15
:✅ Verification successful
Removed unused
imagePriority
binding for compact cards
SinceArticleCompactCardComponent
no longer defines an@Input() imagePriority
, dropping this binding prevents template errors.Please verify that the
imagePriority
input was fully removed from the compact card component’s definition to avoid any dangling references:
🏁 Script executed:
#!/bin/bash # Verify removal of `imagePriority` Input from compact card component rg -n "Input.*imagePriority" -A2 libs/blog/articles/ui-article-card/src/lib/components/article-compact-card rg -n "imagePriority" -A2 libs/blog/articles/ui-article-card/src/lib/ui-article-cardLength of output: 1096
Verification Complete:
imagePriority
input fully removed from compact card component
- Confirmed no
@Input() imagePriority
remains inlibs/blog/articles/ui-article-card/src/lib/components/article-compact-card
- Confirmed no
[imagePriority]
bindings on<al-article-compact-card>
inui-article-card.component.html
libs/blog/become-author/feature-become-author-page/src/lib/become-author-page/become-author-page.component.html (2)
34-34
: Styling simplified by removing text-white classThis change aligns with the PR's objective to revert light mode support, removing explicit white text styling and letting the text color be determined by the theme.
64-64
: Styling simplified by removing text-white classThis change is consistent with the removal of explicit text color classes throughout the application as part of reverting light mode support.
libs/blog/become-author/feature-become-author-page/src/lib/components/become-author-list-item/become-author-list-item.component.html (3)
4-5
: Removed trailing comma in ngClass objectThis is a minor syntax improvement that maintains consistent code style without affecting functionality.
11-11
: Styling simplified by removing text-white classThis change aligns with the PR's objective of reverting light mode support by removing explicit text color classes.
15-15
: Added trailing space in class attributeThis minor formatting change improves readability without affecting functionality.
libs/blog/layouts/ui-navigation/src/lib/navigation/navigation.component.html (3)
9-9
: Removed justify-center class from navigation listThe removal of
justify-center
from the ul element will change how navigation items are aligned, potentially affecting the layout.Verify that this change doesn't negatively impact the navigation's appearance, particularly in horizontal layout mode where center alignment might have been intentional.
24-24
: Replaced conditional text styling with static foreground classThis change simplifies the component by removing conditional text color logic, aligning with the PR's objective to revert light mode support.
36-36
: Replaced conditional text styling with static foreground classThis change is consistent with the other navigation links, replacing conditional text coloring with a static foreground class.
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.ts (2)
12-12
: Removed NgOptimizedImage from importsThe removal of the NgOptimizedImage directive is consistent with the related template changes that replace the optimized image element with CSS background styling.
15-17
: Removed imagePriority input propertyThe removal of the imagePriority input property simplifies the component's API and is consistent with the removal of the optimized image implementation from the template.
libs/blog/authors/ui-author-card/src/lib/author-card/author-card-template.component.ts (1)
22-22
: Styling simplified by removing theme-specific classes.The class has been appropriately modified to remove light/dark mode specific styling, using unified background and border styles. This change aligns with the PR objective of reverting the light mode feature.
libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.scss (4)
13-15
: Added new copyright grid area.The copyright grid area is properly defined to accommodate the copyright text in the footer layout.
19-22
: Updated grid template to include copyright row.The grid template areas are correctly updated to include a third row for copyright that spans the full width in the default layout.
24-31
: Added responsive layout for medium screens.The medium screen breakpoint correctly positions the copyright in a dedicated row while maintaining appropriate layout for other elements.
33-33
: Changed breakpoint from lg to xl.The breakpoint has been updated from
@screen lg
to@screen xl
to align with the responsive design requirements.libs/blog/layouts/ui-layouts/src/lib/footer/components/footer-logo.component.ts (3)
2-2
: Updated import to use the newer Angular signal-based input API.The import statement correctly includes the
input
function from '@angular/core' for the signal-based input API.
19-21
: Added copyright information display.The
<small>
element properly displays the copyright information with the current year, conditionally visible only on larger screens (lg:block
).
26-28
: Added required currentYear input.The component correctly implements a required
currentYear
input using the signal-based input API, which is used to display the copyright year.libs/blog/layouts/ui-layouts/src/lib/footer/footer.component.html (3)
1-1
: Updated footer background to use CSS variable.The footer's background color has been appropriately changed from a specific hex color to the
bg-al-card
CSS variable class, aligning with the theme simplification.
5-6
: Updated component bindings.The changes include:
- Added the required
[currentYear]
input to the footer logo component- Removed the
[whiteFont]
input from the navigation componentThese changes correctly align with the component modifications and theme simplification.
13-15
: Added responsive copyright display for smaller screens.The copyright text is now properly displayed in a dedicated
<small>
element with thecopyright
class, which is visible only on smaller screens (lg:hidden
). This complements the copyright display in the logo component that's visible on larger screens.libs/blog/articles/ui-article-card/src/lib/components/article-regular-card/article-regular-card.component.html (4)
3-3
: Simplified card background and hover stylingThe class has been simplified by removing dark mode specific styles, which aligns with the broader PR objective of removing light theme support.
21-22
: Simplified avatar implementationThe
priority
attribute has been removed from the avatar component, consistent with the removal of image loading priority attributes throughout the codebase.
37-37
: Simplified heading stylesText color classes like
text-al-primary-foreground
have been removed to standardize styling across components.
40-40
: Simplified paragraph stylesText color classes have been removed for consistent styling, retaining only the functional
line-clamp-2
class to limit text display.libs/blog/shared/ui-difficulty/src/lib/ui-difficulty.component.ts (3)
28-30
: Removed dark mode specific classesDark mode prefixes have been removed from background classes, simplifying the conditional styling to use only standard background classes based on the
isColorBackground()
computed property.
47-49
: Standardized shadow stylingDark mode specific shadow classes have been removed, maintaining only the conditional logic for applying different shadows based on the background color setting.
79-81
: Simplified right block stylingSimilar to the other blocks, dark mode specific classes have been removed from the right block styling, maintaining consistency across all parts of the component.
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html (2)
3-8
: Replaced image element with background imageThe implementation has been significantly changed from using an
<img>
element to applying the image directly as a background to the<article>
element. This approach simplifies the markup and aligns with the removal ofNgOptimizedImage
import andimagePriority
property in the component.
11-13
: Updated container background stylingThe inner container now uses a semantic background color class with 85% opacity, replacing previous text color styling. This provides better visual consistency while maintaining the card's content legibility over the background image.
libs/blog/shared/ui-card/src/lib/card.component.ts (2)
72-72
: Updated gradient card background stylingThe
GradientCardDirective
now includes a border class and uses a forced radial gradient background with a fallback background color, removing dark mode specific background classes. This simplifies the styling while maintaining the visual appearance.
100-100
: Removed background class from card componentThe
bg-al-card
class has been removed from the host element, leaving only structural classes like block, rounded, border, shadow, and overflow. This aligns with the pattern of removing theme-specific background classes across components.tailwind.preset.js (3)
3-3
: Explicit dark mode strategy definedThe configuration now explicitly sets Tailwind's dark mode strategy to use class-based approach. This is consistent with the PR objective of reverting light mode support, consolidating to a single theming approach.
27-27
: Box shadow updated to use border variableThe box shadow for 'al-full-border' has been updated to use the
--border
CSS variable instead of what was likely--grey
previously, aligning with the color variable consolidation in the theme reversion.
31-31
: Simplified plugin configurationThe plugins array has been simplified to only include '@tailwindcss/container-queries', removing the theme variants plugin. This change is appropriate since theme variants are no longer needed after reverting light mode support.
@Component({ | ||
selector: 'al-article-compact-card', | ||
changeDetection: ChangeDetectionStrategy.OnPush, | ||
standalone: true, | ||
imports: [ | ||
AvatarComponent, | ||
RouterLink, | ||
AlLocalizePipe, | ||
FastSvgComponent, | ||
NgOptimizedImage, | ||
], | ||
imports: [AvatarComponent, RouterLink, AlLocalizePipe, FastSvgComponent], | ||
templateUrl: './article-compact-card.component.html', | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Component is no longer standalone
The component decorator no longer includes standalone: true
. This change may require updating how this component is registered and consumed in the application.
Verify that this component is properly declared in a NgModule since it's no longer standalone. This change could potentially break module dependencies if not properly handled.
🏁 Script executed:
#!/bin/bash
# Check if the component is properly declared in a module
echo "Checking if ArticleCompactCardComponent is declared in a module..."
grep -r "ArticleCompactCardComponent" --include="*.ts" | grep -i "declarations\|exports"
Length of output: 231
Ensure ArticleCompactCardComponent is registered in an NgModule
You’ve removed standalone: true
from ArticleCompactCardComponent
, so it must now be declared in an NgModule. I didn’t find any references to it in a declarations
or exports
array:
• Add it to the appropriate module (for example libs/blog/articles/ui-article-card/ui-article-card.module.ts
):
@NgModule({
declarations: [ArticleCompactCardComponent, /* … */],
exports: [ArticleCompactCardComponent, /* … */],
imports: [/* … */],
})
export class UiArticleCardModule {}
Without this, consumers of al-article-compact-card
will break at runtime.
This reverts commit 600f623.
Summary by CodeRabbit
New Features
Style
Chores