feat(angular-talks): add list of latest angular talks to home page#545
Open
dawidg-hoa wants to merge 5 commits into
Open
feat(angular-talks): add list of latest angular talks to home page#545dawidg-hoa wants to merge 5 commits into
dawidg-hoa wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds video preview contracts, server endpoints, Angular video data and UI libraries, and homepage rendering for a latest videos section. It also replaces the older article-list title component with a shared section-title component and adds matching translations, aliases, and locator updates. ChangesVideos homepage feature
Sequence Diagram(s)sequenceDiagram
participant HomePage
participant LatestVideosList
participant VideosAPI
participant WordPress
participant VideoDialog
HomePage->>LatestVideosList: render latest videos section
LatestVideosList->>VideosAPI: GET /videos?take=3
VideosAPI->>WordPress: fetch yt-video entries
WordPress-->>VideosAPI: video items + x-wp-total
VideosAPI-->>LatestVideosList: ArrayResponse<VideoPreview>
LatestVideosList->>VideoDialog: open selected video
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
dawidg-hoa
force-pushed
the
feat/angular-talks
branch
from
35d3234 to
3142f09
Compare
|
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 |
|
Deployed to dev environment |
|
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: 3
🧹 Nitpick comments (2)
libs/blog/videos/ui-video-card/src/lib/types/video-card.ts (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid duplicating the shared video contract shape.
VideoCardcurrently mirrorsVideoPreviewexactly, which risks future drift. Reuse the shared contract type (alias/extends) to keep server/client shape aligned.Proposed refactor
+import { VideoPreview } from '`@angular-love/blog/contracts/videos`'; + -export interface VideoCard { - videoId: string; - title: string; - eventName: string; -} +export type VideoCard = VideoPreview;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/blog/videos/ui-video-card/src/lib/types/video-card.ts` around lines 1 - 5, `VideoCard` is duplicating the exact shape of the shared video contract, which can drift over time. Update the `VideoCard` type in the `video-card` types module to reuse the existing shared contract type used by `VideoPreview` instead of redefining the same fields, either by aliasing or extending that shared type so both stay aligned.libs/blog/videos/ui-video-card/src/lib/components/video-card/video-card.stories.ts (1)
41-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove unused
videoargs from the Skeleton story.
VideoCardSkeletonComponentdoesn’t consumevideo, so this arg only adds noise in Storybook controls and can confuse the story contract.Proposed cleanup
export const Skeleton: StoryObj<VideoCardSkeletonComponent> = { - args: { - video: video, - }, render: (args) => ({ props: args, template: ` <div style="width: 405px;"> <al-video-card-skeleton /> </div> `, }), };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/blog/videos/ui-video-card/src/lib/components/video-card/video-card.stories.ts` around lines 41 - 44, The Skeleton story for VideoCardSkeletonComponent is passing an unused video arg, which only adds noise to Storybook controls. Update the Skeleton story in video-card.stories.ts to remove the video entry from the args object and keep the story aligned with the actual component contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/blog-bff/videos/api/src/lib/api.ts`:
- Line 25: The `total` field assignment using
`Number(response.headers.get('x-wp-total'))` lacks validation and can silently
produce NaN or incorrect values when the header is malformed, missing, or
invalid. Replace this with guarded parsing that explicitly checks if the header
exists and contains a valid numeric value, using a deterministic fallback value
(such as 0) when parsing fails or the header is absent. This ensures the total
field always has a valid, predictable value even with malformed response
headers.
In
`@libs/blog/videos/ui-video-dialog/src/lib/components/video-dialog/video-dialog.component.scss`:
- Around line 2-5: The stylesheet animation name is not following the enforced
kebab-case Stylelint rule, so update the animation reference and the matching
`@keyframes` identifier in video-dialog.component.scss to use a kebab-case name.
Keep the animation declaration and the keyframes block in sync by renaming both
symbols together so the existing animation behavior in the video dialog remains
unchanged.
In
`@libs/blog/videos/ui-youtube-video-player/src/lib/components/youtube-video-player/youtube-video-player.component.ts`:
- Around line 31-36: The `lang` variable is being interpolated directly into the
YouTube embed URL query parameters without encoding, which can lead to query
parameter injection vulnerabilities and malformed embeds. In the `videoSrc`
computed property, encode the `lang` variable using `encodeURIComponent` before
interpolating it into the hl and cc_lang_pref query parameters, applying the
same encoding pattern already used for `videoId`.
---
Nitpick comments:
In
`@libs/blog/videos/ui-video-card/src/lib/components/video-card/video-card.stories.ts`:
- Around line 41-44: The Skeleton story for VideoCardSkeletonComponent is
passing an unused video arg, which only adds noise to Storybook controls. Update
the Skeleton story in video-card.stories.ts to remove the video entry from the
args object and keep the story aligned with the actual component contract.
In `@libs/blog/videos/ui-video-card/src/lib/types/video-card.ts`:
- Around line 1-5: `VideoCard` is duplicating the exact shape of the shared
video contract, which can drift over time. Update the `VideoCard` type in the
`video-card` types module to reuse the existing shared contract type used by
`VideoPreview` instead of redefining the same fields, either by aliasing or
extending that shared type so both stay aligned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 445b3f90-084b-4a66-a0a4-bc96ff2dcb27
⛔ Files ignored due to path filters (2)
apps/blog-analog/public/assets/icons/play.svgis excluded by!**/*.svgapps/blog/src/assets/icons/play.svgis excluded by!**/*.svg
📒 Files selected for processing (109)
apps/blog-analog/src/app/i18n/assets/en.jsonapps/blog-analog/src/app/i18n/assets/pl.jsonapps/blog-analog/src/server/routes/api/videos/index.get.tsapps/blog-bff/src/main.tsapps/blog-bff/wrangler.tomlapps/blog-e2e/src/lib/elements/category-container.element.tsapps/blog/src/assets/i18n/en.jsonapps/blog/src/assets/i18n/pl.jsonlibs/blog-bff/videos/api/.eslintrc.jsonlibs/blog-bff/videos/api/README.mdlibs/blog-bff/videos/api/jest.config.tslibs/blog-bff/videos/api/project.jsonlibs/blog-bff/videos/api/src/index.tslibs/blog-bff/videos/api/src/lib/api.tslibs/blog-bff/videos/api/src/lib/dtos.tslibs/blog-bff/videos/api/src/lib/mappers.tslibs/blog-bff/videos/api/src/lib/wp-videos.tslibs/blog-bff/videos/api/tsconfig.jsonlibs/blog-bff/videos/api/tsconfig.lib.jsonlibs/blog-bff/videos/api/tsconfig.spec.jsonlibs/blog-contracts/videos/.eslintrc.jsonlibs/blog-contracts/videos/README.mdlibs/blog-contracts/videos/jest.config.tslibs/blog-contracts/videos/project.jsonlibs/blog-contracts/videos/src/index.tslibs/blog-contracts/videos/src/lib/video-preview.tslibs/blog-contracts/videos/tsconfig.jsonlibs/blog-contracts/videos/tsconfig.lib.jsonlibs/blog-contracts/videos/tsconfig.spec.jsonlibs/blog/articles/feature-list/src/lib/category-section-container/category-section-container.component.htmllibs/blog/articles/feature-list/src/lib/category-section-container/category-section-container.component.tslibs/blog/articles/ui-article-list-title/README.mdlibs/blog/articles/ui-article-list-title/src/index.tslibs/blog/home/feature-home/src/lib/home-page/home-page.component.htmllibs/blog/home/feature-home/src/lib/home-page/home-page.component.tslibs/blog/shared/ui-section-title/.eslintrc.jsonlibs/blog/shared/ui-section-title/README.mdlibs/blog/shared/ui-section-title/jest.config.tslibs/blog/shared/ui-section-title/project.jsonlibs/blog/shared/ui-section-title/src/index.tslibs/blog/shared/ui-section-title/src/lib/ui-section-title/ui-section-title.component.htmllibs/blog/shared/ui-section-title/src/lib/ui-section-title/ui-section-title.component.tslibs/blog/shared/ui-section-title/src/test-setup.tslibs/blog/shared/ui-section-title/tsconfig.jsonlibs/blog/shared/ui-section-title/tsconfig.lib.jsonlibs/blog/shared/ui-section-title/tsconfig.spec.jsonlibs/blog/videos/data-access/.eslintrc.jsonlibs/blog/videos/data-access/README.mdlibs/blog/videos/data-access/jest.config.tslibs/blog/videos/data-access/project.jsonlibs/blog/videos/data-access/src/index.tslibs/blog/videos/data-access/src/lib/dto/videos.query.tslibs/blog/videos/data-access/src/lib/infrastructure/videos.service.tslibs/blog/videos/data-access/src/lib/state/videos-list.store.tslibs/blog/videos/data-access/src/test-setup.tslibs/blog/videos/data-access/tsconfig.jsonlibs/blog/videos/data-access/tsconfig.lib.jsonlibs/blog/videos/data-access/tsconfig.spec.jsonlibs/blog/videos/feature-list/.eslintrc.jsonlibs/blog/videos/feature-list/README.mdlibs/blog/videos/feature-list/jest.config.tslibs/blog/videos/feature-list/project.jsonlibs/blog/videos/feature-list/src/index.tslibs/blog/videos/feature-list/src/lib/components/latest-videos-list-container/latest-videos-list-container.component.htmllibs/blog/videos/feature-list/src/lib/components/latest-videos-list-container/latest-videos-list-container.component.tslibs/blog/videos/feature-list/src/lib/services/video-dialog-manager.service.tslibs/blog/videos/feature-list/src/test-setup.tslibs/blog/videos/feature-list/tsconfig.jsonlibs/blog/videos/feature-list/tsconfig.lib.jsonlibs/blog/videos/feature-list/tsconfig.spec.jsonlibs/blog/videos/ui-video-card/.eslintrc.jsonlibs/blog/videos/ui-video-card/README.mdlibs/blog/videos/ui-video-card/jest.config.tslibs/blog/videos/ui-video-card/project.jsonlibs/blog/videos/ui-video-card/src/index.tslibs/blog/videos/ui-video-card/src/lib/components/video-card/video-card-skeleton.component.tslibs/blog/videos/ui-video-card/src/lib/components/video-card/video-card.component.htmllibs/blog/videos/ui-video-card/src/lib/components/video-card/video-card.component.tslibs/blog/videos/ui-video-card/src/lib/components/video-card/video-card.stories.tslibs/blog/videos/ui-video-card/src/lib/types/video-card.tslibs/blog/videos/ui-video-card/src/test-setup.tslibs/blog/videos/ui-video-card/tsconfig.jsonlibs/blog/videos/ui-video-card/tsconfig.lib.jsonlibs/blog/videos/ui-video-card/tsconfig.spec.jsonlibs/blog/videos/ui-video-dialog/.eslintrc.jsonlibs/blog/videos/ui-video-dialog/README.mdlibs/blog/videos/ui-video-dialog/jest.config.tslibs/blog/videos/ui-video-dialog/project.jsonlibs/blog/videos/ui-video-dialog/src/index.tslibs/blog/videos/ui-video-dialog/src/lib/components/video-dialog/video-dialog.component.htmllibs/blog/videos/ui-video-dialog/src/lib/components/video-dialog/video-dialog.component.scsslibs/blog/videos/ui-video-dialog/src/lib/components/video-dialog/video-dialog.component.tslibs/blog/videos/ui-video-dialog/src/lib/types/video-dialog-data.tslibs/blog/videos/ui-video-dialog/src/test-setup.tslibs/blog/videos/ui-video-dialog/tsconfig.jsonlibs/blog/videos/ui-video-dialog/tsconfig.lib.jsonlibs/blog/videos/ui-video-dialog/tsconfig.spec.jsonlibs/blog/videos/ui-youtube-video-player/.eslintrc.jsonlibs/blog/videos/ui-youtube-video-player/README.mdlibs/blog/videos/ui-youtube-video-player/jest.config.tslibs/blog/videos/ui-youtube-video-player/project.jsonlibs/blog/videos/ui-youtube-video-player/src/index.tslibs/blog/videos/ui-youtube-video-player/src/lib/components/youtube-video-player/youtube-video-player.component.htmllibs/blog/videos/ui-youtube-video-player/src/lib/components/youtube-video-player/youtube-video-player.component.tslibs/blog/videos/ui-youtube-video-player/src/test-setup.tslibs/blog/videos/ui-youtube-video-player/tsconfig.jsonlibs/blog/videos/ui-youtube-video-player/tsconfig.lib.jsonlibs/blog/videos/ui-youtube-video-player/tsconfig.spec.jsontsconfig.base.json
💤 Files with no reviewable changes (2)
- libs/blog/articles/ui-article-list-title/README.md
- libs/blog/articles/ui-article-list-title/src/index.ts
|
|
||
| return c.json(<ArrayResponse<VideoPreview>>{ | ||
| data: response.data.map(mapWPVideoToVideoPreview), | ||
| total: Number(response.headers.get('x-wp-total')), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Harden total parsing from x-wp-total.
Number(response.headers.get('x-wp-total')) can silently yield an invalid/incorrect total value for malformed headers. Prefer guarded parsing with a deterministic fallback.
Proposed fix
+ const totalHeader = response.headers.get('x-wp-total');
+ const parsedTotal = totalHeader ? Number.parseInt(totalHeader, 10) : NaN;
+
return c.json(<ArrayResponse<VideoPreview>>{
data: response.data.map(mapWPVideoToVideoPreview),
- total: Number(response.headers.get('x-wp-total')),
+ total: Number.isFinite(parsedTotal) ? parsedTotal : response.data.length,
});📝 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.
Suggested change
| total: Number(response.headers.get('x-wp-total')), | |
| const totalHeader = response.headers.get('x-wp-total'); | |
| const parsedTotal = totalHeader ? Number.parseInt(totalHeader, 10) : NaN; | |
| return c.json(<ArrayResponse<VideoPreview>>{ | |
| data: response.data.map(mapWPVideoToVideoPreview), | |
| total: Number.isFinite(parsedTotal) ? parsedTotal : response.data.length, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/blog-bff/videos/api/src/lib/api.ts` at line 25, The `total` field
assignment using `Number(response.headers.get('x-wp-total'))` lacks validation
and can silently produce NaN or incorrect values when the header is malformed,
missing, or invalid. Replace this with guarded parsing that explicitly checks if
the header exists and contains a valid numeric value, using a deterministic
fallback value (such as 0) when parsing fails or the header is absent. This
ensures the total field always has a valid, predictable value even with
malformed response headers.
Comment on lines
+2
to
+5
| animation: fadeInBottom 0.4s ease-out forwards; | ||
| } | ||
|
|
||
| @keyframes fadeInBottom { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the animation keyframes to kebab-case.
fadeInBottom will trip the configured Stylelint rule. Please rename both the animation reference and the @keyframes block so the stylesheet stays lint-clean.
Suggested fix
.dialog {
- animation: fadeInBottom 0.4s ease-out forwards;
+ animation: fade-in-bottom 0.4s ease-out forwards;
}
-@keyframes fadeInBottom {
+@keyframes fade-in-bottom {📝 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.
Suggested change
| animation: fadeInBottom 0.4s ease-out forwards; | |
| } | |
| @keyframes fadeInBottom { | |
| .dialog { | |
| animation: fade-in-bottom 0.4s ease-out forwards; | |
| } | |
| `@keyframes` fade-in-bottom { |
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 5-5: Expected keyframe name "fadeInBottom" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@libs/blog/videos/ui-video-dialog/src/lib/components/video-dialog/video-dialog.component.scss`
around lines 2 - 5, The stylesheet animation name is not following the enforced
kebab-case Stylelint rule, so update the animation reference and the matching
`@keyframes` identifier in video-dialog.component.scss to use a kebab-case name.
Keep the animation declaration and the keyframes block in sync by renaming both
symbols together so the existing animation behavior in the video dialog remains
unchanged.
Source: Linters/SAST tools
Comment on lines
+31
to
+36
| protected readonly videoSrc = computed(() => { | ||
| const lang = this.lang(); | ||
| const videoId = encodeURIComponent(this.videoId()); | ||
| return this.domSanitizer.bypassSecurityTrustResourceUrl( | ||
| `https://www.youtube-nocookie.com/embed/${videoId}?rel=0&autoplay=1&hl=${lang}&cc_lang_pref=${lang}`, | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Encode locale before interpolating into trusted embed URL.
Line 35 interpolates lang directly into query params while using bypassSecurityTrustResourceUrl; encode it first to prevent query-parameter injection and malformed embeds.
Proposed fix
protected readonly videoSrc = computed(() => {
- const lang = this.lang();
+ const lang = encodeURIComponent(this.lang());
const videoId = encodeURIComponent(this.videoId());
return this.domSanitizer.bypassSecurityTrustResourceUrl(
`https://www.youtube-nocookie.com/embed/${videoId}?rel=0&autoplay=1&hl=${lang}&cc_lang_pref=${lang}`,
);
});📝 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.
Suggested change
| protected readonly videoSrc = computed(() => { | |
| const lang = this.lang(); | |
| const videoId = encodeURIComponent(this.videoId()); | |
| return this.domSanitizer.bypassSecurityTrustResourceUrl( | |
| `https://www.youtube-nocookie.com/embed/${videoId}?rel=0&autoplay=1&hl=${lang}&cc_lang_pref=${lang}`, | |
| ); | |
| protected readonly videoSrc = computed(() => { | |
| const lang = encodeURIComponent(this.lang()); | |
| const videoId = encodeURIComponent(this.videoId()); | |
| return this.domSanitizer.bypassSecurityTrustResourceUrl( | |
| `https://www.youtube-nocookie.com/embed/${videoId}?rel=0&autoplay=1&hl=${lang}&cc_lang_pref=${lang}`, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@libs/blog/videos/ui-youtube-video-player/src/lib/components/youtube-video-player/youtube-video-player.component.ts`
around lines 31 - 36, The `lang` variable is being interpolated directly into
the YouTube embed URL query parameters without encoding, which can lead to query
parameter injection vulnerabilities and malformed embeds. In the `videoSrc`
computed property, encode the `lang` variable using `encodeURIComponent` before
interpolating it into the hl and cc_lang_pref query parameters, applying the
same encoding pattern already used for `videoId`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add new section to home page with latest angular talks.
The list of three latest videos is fetched from word press, which are then displayed as preview cards at the bottom of the home page. If user clicks the preview card a dialog is displayed with embedded youtube player.
Changes:
article-list-titlecomponent into a more generalsection-titlecomponent as the same UI is needed also for videos list section. Pretty much only a rename and location change was needed as it wasn't coupled with articles.Summary by CodeRabbit
Release Notes
New Features
Internationalization
UI/UX Improvements