-
Notifications
You must be signed in to change notification settings - Fork 10
feat/light-mode #444
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
base: main
Are you sure you want to change the base?
feat/light-mode #444
Changes from 10 commits
d52b3b1
956f02b
c8ace7e
38ca2d3
e97641f
9329dcd
0445dc3
e0a2694
5fccbda
e957263
18a4c96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,16 +1,17 @@ | ||||||||||||||||||||||||||||||||||||
<a [routerLink]="['/', article().slug] | alLocalize"> | ||||||||||||||||||||||||||||||||||||
<article | ||||||||||||||||||||||||||||||||||||
class="h-full rounded-lg bg-cover bg-no-repeat transition-transform hover:scale-105 motion-reduce:transition-none motion-reduce:hover:scale-100" | ||||||||||||||||||||||||||||||||||||
[style.background-image]=" | ||||||||||||||||||||||||||||||||||||
'url(' + | ||||||||||||||||||||||||||||||||||||
(article().featuredImageUrl || 'assets/article-placeholder.webp') + | ||||||||||||||||||||||||||||||||||||
')' | ||||||||||||||||||||||||||||||||||||
" | ||||||||||||||||||||||||||||||||||||
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" | ||||||||||||||||||||||||||||||||||||
[attr.aria-labelledby]="article().slug" | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||||
class="bg-al-background relative flex h-full flex-col justify-between opacity-85" | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
<img | ||||||||||||||||||||||||||||||||||||
alt="Post featured image" | ||||||||||||||||||||||||||||||||||||
class="absolute h-auto w-full bg-contain opacity-20" | ||||||||||||||||||||||||||||||||||||
[ngSrc]="article().featuredImageUrl || 'assets/article-placeholder.webp'" | ||||||||||||||||||||||||||||||||||||
[priority]="imagePriority()" | ||||||||||||||||||||||||||||||||||||
width="1215" | ||||||||||||||||||||||||||||||||||||
height="750" | ||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||
Comment on lines
+6
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolute image needs correct positioning and object-fit; fix wrong Tailwind class; clarify accessibility; ensure priority boolean
Apply this diff: - <img
- alt="Post featured image"
- class="absolute h-auto w-full bg-contain opacity-20"
- [ngSrc]="article().featuredImageUrl || 'assets/article-placeholder.webp'"
- [priority]="imagePriority()"
- width="1215"
- height="750"
- />
+ <img
+ alt=""
+ aria-hidden="true"
+ class="absolute inset-0 z-0 h-full w-full object-cover opacity-20"
+ [ngSrc]="article().featuredImageUrl || 'assets/article-placeholder.webp'"
+ [priority]="!!imagePriority()"
+ width="1215"
+ height="750"
+ /> Notes:
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||
<div class="relative flex h-full flex-col justify-between text-[#fff]"> | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Text color hard-coded; add stacking safety for content layer
Apply this diff to enforce stacking (and drop hard-coded hex if tokens exist): - <div class="relative flex h-full flex-col justify-between text-[#fff]">
+ <div class="relative z-20 flex h-full flex-col justify-between text-al-foreground"> If <!-- Line 16 update example -->
<div class="absolute inset-0 z-10 h-full w-full pointer-events-none hover:bg-al-bottom-radial-gradient"></div> |
||||||||||||||||||||||||||||||||||||
<div | ||||||||||||||||||||||||||||||||||||
class="hover:bg-al-bottom-radial-gradient absolute h-full w-full" | ||||||||||||||||||||||||||||||||||||
></div> | ||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,6 @@ | ||||||
<a [routerLink]="['/', article().slug] | alLocalize"> | ||||||
<article | ||||||
class="bg-al-card md:hover:shadow-al-primary group relative h-full w-full rounded-lg border border-transparent shadow-none transition-transform motion-reduce:transition-none lg:hover:scale-105 lg:motion-reduce:hover:scale-100" | ||||||
class="bg-al-card md:hover:shadow-al-primary group relative h-full w-full rounded-lg border shadow-none transition-transform motion-reduce:transition-none lg:hover:scale-105 lg:motion-reduce:hover:scale-100 dark:border-transparent" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Inconsistent border approach between themes. The border styling uses - class="bg-al-card md:hover:shadow-al-primary group relative h-full w-full rounded-lg border shadow-none transition-transform motion-reduce:transition-none lg:hover:scale-105 lg:motion-reduce:hover:scale-100 dark:border-transparent"
+ class="bg-al-card md:hover:shadow-al-primary group relative h-full w-full rounded-lg border-al-border shadow-none transition-transform motion-reduce:transition-none lg:hover:scale-105 lg:motion-reduce:hover:scale-100" 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||
[attr.aria-labelledby]="article().slug" | ||||||
> | ||||||
<div | ||||||
|
@@ -18,8 +18,12 @@ | |||||
<div class="rounded-b-lg"> | ||||||
<div class="flex items-center justify-between px-4 pt-4"> | ||||||
<div class="flex items-center gap-2"> | ||||||
<al-avatar [imageSrc]="article().author.avatarUrl" size="32" /> | ||||||
<span class="text-sm/[14px] font-medium"> | ||||||
<al-avatar | ||||||
[imageSrc]="article().author.avatarUrl" | ||||||
[priority]="imagePriority()" | ||||||
size="32" | ||||||
/> | ||||||
<span class="text-al-primary-foreground text-sm/[14px] font-medium"> | ||||||
{{ article().author.name }} | ||||||
</span> | ||||||
</div> | ||||||
|
@@ -34,10 +38,14 @@ | |||||
</div> | ||||||
</div> | ||||||
<div class="flex flex-col gap-3 px-4 pb-4 pt-3"> | ||||||
<h3 class="text-2xl font-bold" [id]="article().slug"> | ||||||
<h3 | ||||||
class="*:text-al-primary-foreground text-2xl font-bold *:not-italic" | ||||||
[id]="article().slug" | ||||||
style="word-break: break-word" | ||||||
> | ||||||
{{ article().title }} | ||||||
</h3> | ||||||
<p class="line-clamp-2"> | ||||||
<p class="*:text-al-pink line-clamp-2 *:font-medium *:not-italic"> | ||||||
{{ article().excerpt }} | ||||||
</p> | ||||||
</div> | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,4 +1,4 @@ | ||||||||||||||
import { Component } from '@angular/core'; | ||||||||||||||
import { Component, computed, input } from '@angular/core'; | ||||||||||||||
|
||||||||||||||
import { | ||||||||||||||
CardComponent, | ||||||||||||||
|
@@ -13,29 +13,43 @@ import { | |||||||||||||
class: 'block @container', | ||||||||||||||
}, | ||||||||||||||
template: ` | ||||||||||||||
<al-card alGradientCard> | ||||||||||||||
<al-card alGradientCard [hideGradient]="hideGradient()"> | ||||||||||||||
<div alCardContent> | ||||||||||||||
<div | ||||||||||||||
class="@3xl:flex-row @3xl:border-none flex w-full flex-col items-center rounded-lg border" | ||||||||||||||
class="flex w-full flex-col items-center rounded-lg border" | ||||||||||||||
[class]="cardWrapper()" | ||||||||||||||
> | ||||||||||||||
<div | ||||||||||||||
class="@3xl:border @3xl:!bg-al-radial-gradient @3xl:bg-al-background @3xl:min-w-[260px] min-w-fit rounded-lg pb-4 pt-6" | ||||||||||||||
class="min-w-fit rounded-lg pt-6 md:min-w-[260px]" | ||||||||||||||
[class]="authorInfoCardClass()" | ||||||||||||||
> | ||||||||||||||
<div | ||||||||||||||
class="@3xl:max-w-[360px] flex w-full flex-col items-center gap-4" | ||||||||||||||
class="flex w-full flex-col items-center gap-4 md:max-w-[360px]" | ||||||||||||||
Expand | ||||||||||||||
Down | ||||||||||||||
> | ||||||||||||||
Comment on lines
+28
to
30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove extraneous text nodes from template. The template contains standalone text nodes "Expand" and "Down" that appear to be leftover content or debugging artifacts and should be removed. <div
class="flex w-full flex-col items-center gap-4 md:max-w-[360px]"
- Expand
- Down
> 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||
<ng-content select="[author-info-card]"></ng-content> | ||||||||||||||
</div> | ||||||||||||||
</div> | ||||||||||||||
|
||||||||||||||
<div | ||||||||||||||
class="@3xl:pt-6 w-full flex-1 hyphens-auto break-words p-6 pt-0" | ||||||||||||||
> | ||||||||||||||
<div class="w-full flex-1 hyphens-auto break-words p-6 pt-0 md:pt-6"> | ||||||||||||||
<ng-content select="[author-info-description]"></ng-content> | ||||||||||||||
</div> | ||||||||||||||
</div> | ||||||||||||||
</div> | ||||||||||||||
</al-card> | ||||||||||||||
`, | ||||||||||||||
}) | ||||||||||||||
export class AuthorCardTemplateComponent {} | ||||||||||||||
export class AuthorCardTemplateComponent { | ||||||||||||||
readonly hideGradient = input<boolean>(true); | ||||||||||||||
readonly articleCard = input<boolean>(false); | ||||||||||||||
|
||||||||||||||
protected readonly cardWrapper = computed(() => | ||||||||||||||
!this.articleCard() ? 'md:flex-row md:border-none' : '', | ||||||||||||||
); | ||||||||||||||
|
||||||||||||||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Hard-coded bg-black undermines light-mode theming
Using bg-black forces a dark card in all themes. For light mode, this should be theme-driven (CSS vars or theme variants), not a fixed color.
Consider replacing with a theme-aware class/token (examples, adjust to your setup):
bg-white text-gray-900 [data-theme=dark]:bg-black [data-theme=dark]:text-white
bg-al-background text-al-foreground
Would you like me to generate a patch once you confirm the preferred tokens/variants?
🏁 Script executed:
Length of output: 9683
Hard-coded bg-black breaks light-mode theming
Replace the fixed black background with your project’s theme tokens so the card adapts to both light and dark modes. For example, to match the regular card component you could use the
al-card
token:• File:
libs/blog/articles/ui-article-card/src/lib/components/article-compact-card/article-compact-card.component.html
• Lines: 3
Suggested diff:
If you need a distinct dark-mode override, you can also add a
dark:
variant (e.g.dark:bg-al-background
).📝 Committable suggestion
🤖 Prompt for AI Agents