Uh oh!
There was an error while loading. Please reload this page.
Add distribution status column - #347
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a new Distribution Status column to the analyses table UI with badge rendering and tooltip; centers the Run Status header; adjusts CSS for distribution-badge spacing and log card theming; updates unit tests to new column indices and introduces a reactive refresh mock for test data updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/analysis/logs/AnalysisLogCardContent.vue (1)
157-193:⚠️ Potential issue | 🟠 MajorAdd light-mode styling for
.log-scroll-panelto ensure readability in light mode.The
.log-scroll-panelonly has colors defined under.flame-dark(lines 183-186) but not for light mode. In light mode, it inherits the light background from.log-cardwhile keeping the hardcoded light text color (#f1f5f9), resulting in poor contrast.Add explicit light-mode styling:
Suggested fix
.flame-light .log-scroll-panel { background:var(--p-slate-50); color:var(--p-slate-900); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/logs/AnalysisLogCardContent.vue` around lines 157 - 193, The .log-scroll-panel lacks explicit light-mode colors causing poor contrast in light themes; add a .flame-light .log-scroll-panel rule that sets an appropriate light background and dark text (e.g., use --p-slate-50 for background and --p-slate-900 for color) so .log-scroll-panel has explicit styling in both .flame-dark and .flame-light themes and remains readable.
🧹 Nitpick comments (1)
app/components/analysis/AnalysesTable.vue (1)
649-665: Consider extracting repeated badge markup into a shared component or helper.The Distribution Status badge structure (lines 649-665) is nearly identical to the Data Store badge (lines 743-756). Both use Badge with check/times icons, tooltips, and similar severity logic.
This could be extracted to reduce duplication, though it's minor given the scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 649 - 665, Extract the duplicated Badge markup into a small reusable component (e.g., StatusBadge) and replace both occurrences in AnalysesTable.vue: create a StatusBadge component that accepts a boolean/condition prop (e.g., "active" or "condition"), active/inactive tooltip text and optional severity/icon props (defaults: active -> severity="success" + pi-check + "Image available", inactive -> severity="danger" + pi-times + "Image unavailable"), render the same wrapper class ("distribution-badge") and Badge element with v-tooltip and icon based on the condition, then update the distribution_status rendering (where you check data.analysis.distribution_status === 'executed') and the Data Store badge (where you check data.store or similar) to use <StatusBadge :condition="..." :active-tooltip="..." :inactive-tooltip="..."/> so both places reuse the component and preserve existing classes and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 634-667: The Column is marked sortable but uses
field="distribution" while the template reads data.analysis.distribution_status,
so sorting operates on undefined; fix by making the Column's field match the
actual data path (use "analysis.distribution_status" or the exact property used)
or supply a proper sortField/field extractor for the Column component to
reference data.analysis.distribution_status (update the Column declaration where
field="distribution" and ensure :sortable remains true so sorting targets the
real distribution_status value).
---
Outside diff comments:
In `@app/components/analysis/logs/AnalysisLogCardContent.vue`:
- Around line 157-193: The .log-scroll-panel lacks explicit light-mode colors
causing poor contrast in light themes; add a .flame-light .log-scroll-panel rule
that sets an appropriate light background and dark text (e.g., use --p-slate-50
for background and --p-slate-900 for color) so .log-scroll-panel has explicit
styling in both .flame-dark and .flame-light themes and remains readable.
---
Nitpick comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 649-665: Extract the duplicated Badge markup into a small reusable
component (e.g., StatusBadge) and replace both occurrences in AnalysesTable.vue:
create a StatusBadge component that accepts a boolean/condition prop (e.g.,
"active" or "condition"), active/inactive tooltip text and optional
severity/icon props (defaults: active -> severity="success" + pi-check + "Image
available", inactive -> severity="danger" + pi-times + "Image unavailable"),
render the same wrapper class ("distribution-badge") and Badge element with
v-tooltip and icon based on the condition, then update the distribution_status
rendering (where you check data.analysis.distribution_status === 'executed') and
the Data Store badge (where you check data.store or similar) to use <StatusBadge
:condition="..." :active-tooltip="..." :inactive-tooltip="..."/> so both places
reuse the component and preserve existing classes and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63a10d85-0d48-4593-926b-f9ff1f7a208b
📒 Files selected for processing (3)
app/components/analysis/AnalysesTable.vueapp/components/analysis/logs/AnalysisLogCardContent.vuetest/components/analysis/AnalysisTable.spec.ts
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/analysis/AnalysesTable.vue (1)
882-884: Use layout centering instead of a hardcoded left margin.
margin-left: 4remis fragile across viewport/table changes. Center via flex to keep badge placement stable.♻️ CSS refactor
.distribution-badge { - margin-left: 4rem;+ display: flex;+ justify-content: center; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 882 - 884, Replace the fragile hardcoded offset on .distribution-badge with flex-based centering: remove margin-left: 4rem and instead center the badge using flexbox (e.g., make the badge's container a flex row and use justify-content: center and align-items: center, or set .distribution-badge to display:flex with justify-content:center/align-items:center if it is the direct container). Update the CSS rule for .distribution-badge (and its parent container if needed) to use these flex properties so the badge stays centered responsively instead of relying on a fixed left margin.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 650-665: The UI currently collapses every non-executed
distribution_status into a single "unavailable" state; update the rendering in
AnalysesTable.vue to map each possible distribution_status value (use the known
values from Api.ts around distribution_status) to its own Badge severity, icon
and tooltip instead of using a binary executed vs else branch—replace the
v-if/v-else that only checks distribution_status === 'executed' with a small
status-to-metadata mapping (status -> {severity, icon, tooltip}) and render one
Badge using that mapping (keep using v-tooltip and the pi- icons but choose
pi-check, pi-spinner, pi-times, etc. appropriately) so in-progress, stopped,
failed, executed and other statuses show distinct visuals and tooltips.
---
Nitpick comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 882-884: Replace the fragile hardcoded offset on
.distribution-badge with flex-based centering: remove margin-left: 4rem and
instead center the badge using flexbox (e.g., make the badge's container a flex
row and use justify-content: center and align-items: center, or set
.distribution-badge to display:flex with
justify-content:center/align-items:center if it is the direct container). Update
the CSS rule for .distribution-badge (and its parent container if needed) to use
these flex properties so the badge stays centered responsively instead of
relying on a fixed left margin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0df0e4eb-5680-440b-9be1-012dd81fc8fb
📒 Files selected for processing (1)
app/components/analysis/AnalysesTable.vue
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
And fix log colors
Summary by CodeRabbit
New Features
Style
Tests