Tags display#146
Conversation
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 47 minutes and 52 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a new ChangesTag Display Mode Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
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 unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
projects/ngx/declarative-ui/value-cell/tag-list-value/tag-list-value.component.html (1)
2-2: ⚡ Quick winConsider tracking by index instead of value.
The
track tagexpression tracks by value, which may cause rendering issues if thetagsarray contains duplicate strings. Angular could reuse DOM elements incorrectly in such cases.🔄 Suggested improvement
- `@for` (tag of tags(); track tag) { + `@for` (tag of tags(); track $index) {🤖 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/value-cell/tag-list-value/tag-list-value.component.html` at line 2, The template currently tracks items by value using the expression "track tag" in tag-list-value.component.html which breaks with duplicate strings; change tracking to use the item index or an explicit trackBy function instead—either update the loop to track the index (e.g., use the index variable instead of tag: the `@for` expression should track the index like "track i" or "track $index" depending on template syntax) or add a simple trackByIndex method on TagListValueComponent (e.g., trackByIndex(index: number) { return index; }) and reference it in the template (e.g., "track trackByIndex") so DOM identity is stable for duplicate tag values.
🤖 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.
Nitpick comments:
In
`@projects/ngx/declarative-ui/value-cell/tag-list-value/tag-list-value.component.html`:
- Line 2: The template currently tracks items by value using the expression
"track tag" in tag-list-value.component.html which breaks with duplicate
strings; change tracking to use the item index or an explicit trackBy function
instead—either update the loop to track the index (e.g., use the index variable
instead of tag: the `@for` expression should track the index like "track i" or
"track $index" depending on template syntax) or add a simple trackByIndex method
on TagListValueComponent (e.g., trackByIndex(index: number) { return index; })
and reference it in the template (e.g., "track trackByIndex") so DOM identity is
stable for duplicate tag values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 172bcb8e-1db7-4291-b8cc-86bf20ffa475
📒 Files selected for processing (17)
docs/declarative-table.mddocs/value-cell.mdprojects/ngx/declarative-ui/models/ui-definition.tsprojects/ngx/declarative-ui/stories/declarative-table.stories.tsprojects/ngx/declarative-ui/stories/pods-table.config.tsprojects/ngx/declarative-ui/table-card/declarative-table-card.component.htmlprojects/ngx/declarative-ui/table/declarative-table/declarative-table.component.htmlprojects/ngx/declarative-ui/table/declarative-table/declarative-table.component.scssprojects/ngx/declarative-ui/value-cell/index.tsprojects/ngx/declarative-ui/value-cell/tag-list-value/tag-list-value.component.htmlprojects/ngx/declarative-ui/value-cell/tag-list-value/tag-list-value.component.scssprojects/ngx/declarative-ui/value-cell/tag-list-value/tag-list-value.component.spec.tsprojects/ngx/declarative-ui/value-cell/tag-list-value/tag-list-value.component.tsprojects/ngx/declarative-ui/value-cell/value-cell.component.htmlprojects/ngx/declarative-ui/value-cell/value-cell.component.scssprojects/ngx/declarative-ui/value-cell/value-cell.component.spec.tsprojects/ngx/declarative-ui/value-cell/value-cell.component.ts
💤 Files with no reviewable changes (1)
- projects/ngx/declarative-ui/value-cell/value-cell.component.scss
Signed-off-by: Sobyt483 <andrianingomel@gmail.com>
| interface TagSettings { | ||
| design?: 'Neutral' | 'Positive' | 'Critical' | 'Negative' | 'Information' | 'Set1' | 'Set2'; | ||
| colorScheme?: '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' | '10'; // default '1' | ||
| separator?: string; // default ',' |
There was a problem hiding this comment.
I have a temptetion to rename it to valueSeparator, what do you think?
What changed
New
tagdisplay mode inValueCellAdded
displayAs: 'tag'as a new cell rendering option. When set, the cell renders each value as a<ui5-tag>chip instead of plain text.',')tagSettingsconfig:design,colorScheme,separatorNew
TagListValuecomponentIntroduced
TagListValue(mfp-tag-list-value) — a standalone component that wraps an array of strings into a wrapping flex container of<ui5-tag>chips. Exported from the public API alongsideBooleanValue,LinkValue, andSecretValue.New
TagSettingsinterfaceAdded
TagSettingsto the model:Removal of
labelDisplayUiSettings.labelDisplayand the associated.label-valueCSS (blue pill badge) have been removed. Existing usages in stories and config are migrated todisplayAs: 'tag'.Change Log
UiSettings.labelDisplayremoved — replace withuiSettings: { displayAs: 'tag' }displayAs: 'tag'cell rendering mode with<ui5-tag>chip outputTagSettingsinterface (design,colorScheme,separator)TagListValuestandalone component, exported from public APIGroupedWithLabelsAndAlertstory toGroupedWithTagsAndAlertTagsStorybook story demonstrating comma-separated, pipe-separated, and array inputsdeclarative-table.component.htmlanddeclarative-table-card.component.htmlSummary by CodeRabbit
Release Notes
New Features
Documentation
Tests