feat: rework add cards dialog#149
Merged
Merged
Conversation
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces AddCardDialog with EditCardsDialog (add + remove). Dashboard uses the new dialog and onCardsEdited({ added, removed }) handler; tests, Storybook stories, exports, and docs updated. Minor Pods card story/config adjustments applied. ChangesDialog Implementation and Integration
Sequence DiagramsequenceDiagram
participant User
participant Dashboard
participant EditCardsDialog
User->>Dashboard: click "Edit Cards"
Dashboard->>EditCardsDialog: open(availableCards, addedCardsIds)
EditCardsDialog->>EditCardsDialog: init selectedIds from addedCardsIds
User->>EditCardsDialog: toggle selections and click Save
EditCardsDialog->>Dashboard: emit confirm {added, removed}
Dashboard->>Dashboard: remove IDs in removed and append added cards
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
projects/ngx/declarative-ui/dashboard/dashboard/dashboard.component.ts (1)
222-226: ⚡ Quick winPreserve unique card IDs and clear stale positions in
onCardsEdited.Line 225 appends incoming cards blindly, and removed IDs are not purged from
cardsPosition. A defensive merge avoids duplicateidkeys and stalex/yreuse.♻️ Proposed refactor
onCardsEdited(event: { added: CardConfig[]; removed: string[] }): void { + const removedIds = new Set(event.removed); + removedIds.forEach((id) => this.cardsPosition.delete(id)); this.cards.update((list) => { - const withoutRemoved = list.filter((c) => !event.removed.includes(c.id)); - return [...withoutRemoved, ...event.added.map((ac) => ({ ...ac }))]; + const next = list.filter((c) => !removedIds.has(c.id)); + const nextIds = new Set(next.map((c) => c.id)); + const uniqueAdded = event.added + .filter((ac) => !nextIds.has(ac.id)) + .map((ac) => ({ ...ac })); + return [...next, ...uniqueAdded]; }); this.closeCardPanel(); }🤖 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 `@projects/ngx/declarative-ui/dashboard/dashboard/dashboard.component.ts` around lines 222 - 226, onCardsEdited currently appends incoming CardConfig items blindly and never cleans up cardsPosition, causing duplicate ids and stale x/y reuse; modify the cards.update callback to perform a defensive merge: build a map of existing cards by id, remove any entries whose ids are in event.removed, then upsert event.added items by replacing existing entries with the same id (preserving other fields only when not overwritten) so no duplicates remain; additionally, remove any keys from the cardsPosition store that match removed ids and reset/clear positions for replaced ids so old x/y are not reused (referencing onCardsEdited, cards.update, cardsPosition, CardConfig, id, x/y).
🤖 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
`@projects/ngx/declarative-ui/dashboard/edit-cards-dialog/edit-cards-dialog.component.ts`:
- Around line 3-23: The component decorator for EditCardsDialog is missing
standalone mode and OnPush change detection; update the `@Component` to include
standalone: true and changeDetection: ChangeDetectionStrategy.OnPush, add an
import for ChangeDetectionStrategy from '`@angular/core`', and ensure the class
name EditCardsDialog (or the component class) remains unchanged; no other logic
changes needed.
---
Nitpick comments:
In `@projects/ngx/declarative-ui/dashboard/dashboard/dashboard.component.ts`:
- Around line 222-226: onCardsEdited currently appends incoming CardConfig items
blindly and never cleans up cardsPosition, causing duplicate ids and stale x/y
reuse; modify the cards.update callback to perform a defensive merge: build a
map of existing cards by id, remove any entries whose ids are in event.removed,
then upsert event.added items by replacing existing entries with the same id
(preserving other fields only when not overwritten) so no duplicates remain;
additionally, remove any keys from the cardsPosition store that match removed
ids and reset/clear positions for replaced ids so old x/y are not reused
(referencing onCardsEdited, cards.update, cardsPosition, CardConfig, id, x/y).
🪄 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: d50d88cd-b0b9-4335-a968-64857d81690f
📒 Files selected for processing (16)
docs/dashboard.mdprojects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.htmlprojects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.scssprojects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.spec.tsprojects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.tsprojects/ngx/declarative-ui/dashboard/dashboard/dashboard.component.htmlprojects/ngx/declarative-ui/dashboard/dashboard/dashboard.component.spec.tsprojects/ngx/declarative-ui/dashboard/dashboard/dashboard.component.tsprojects/ngx/declarative-ui/dashboard/edit-cards-dialog/edit-cards-dialog.component.htmlprojects/ngx/declarative-ui/dashboard/edit-cards-dialog/edit-cards-dialog.component.scssprojects/ngx/declarative-ui/dashboard/edit-cards-dialog/edit-cards-dialog.component.spec.tsprojects/ngx/declarative-ui/dashboard/edit-cards-dialog/edit-cards-dialog.component.tsprojects/ngx/declarative-ui/dashboard/index.tsprojects/ngx/declarative-ui/stories/dashboard.cards.tsprojects/ngx/declarative-ui/stories/dashboard.stories.tsprojects/ngx/declarative-ui/stories/edit-cards-dialog.stories.ts
💤 Files with no reviewable changes (4)
- projects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.spec.ts
- projects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.html
- projects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.scss
- projects/ngx/declarative-ui/dashboard/add-card-dialog/add-card-dialog.component.ts
gkrajniak
approved these changes
May 26, 2026
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
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.
Summary by CodeRabbit