Skip to content

fix(list): 🐛 move cloud icon to model column#51

Merged
jorben merged 2 commits intomasterfrom
fix/cloud-icon-after-model
Mar 2, 2026
Merged

fix(list): 🐛 move cloud icon to model column#51
jorben merged 2 commits intomasterfrom
fix/cloud-icon-after-model

Conversation

@jorben
Copy link
Collaborator

@jorben jorben commented Mar 2, 2026

Summary

  • Move the cloud task icon from the File column to the Model column in the task list.
  • Fix vertical alignment by centering model text and cloud icon on the same Y axis.
  • Add a renderer test to verify the cloud icon appears in the model cell and not in the file cell.

Test Plan

  • npm run test:renderer -- src/renderer/pages/__tests__/List.test.tsx
  • Pre-commit test suite passed via git hook (unit + renderer)

🤖 Generated with Codex Cli

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR correctly moves the cloud-task indicator from the File column to the Model column and updates layout so icon/text are vertically aligned. The added renderer test validates the intended behavior and catches regressions in column placement. Overall this looks good and is ready to merge.

Important

  • src/renderer/pages/__tests__/List.test.tsx:445Brittle test coupling to column order: The assertions use cells[0] and cells[1], which will break if table column order changes (even if behavior remains correct). Prefer selecting the specific File/Model cells by column class/data attribute or matching cell content first, then asserting cloud icon presence/absence in those identified cells.

Praise

  • src/renderer/pages/List.tsx:540Good UI alignment fix with flex container: Using display: flex + alignItems: center cleanly solves vertical alignment between model text and cloud icon.
  • src/renderer/pages/List.tsx:552Handled truncation with icon coexistence: Reducing text max width when provider is cloud avoids overlap and preserves icon visibility for long model names.
  • src/renderer/pages/__tests__/List.test.tsx:413Targeted regression test added: The new test directly verifies the icon moved to the correct column and is absent from the previous one, which is exactly the right scope for this bug fix.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR primarily moves the cloud-task indicator from the File column to the Model column and adds a renderer test to validate that behavior. It also includes an additional status-tooltip change to surface failure reasons for both local and cloud tasks. Overall this looks solid and close to merge-ready, but there is one correctness issue in the new test that can make it flaky.

Important

  • src/renderer/pages/__tests__/List.test.tsx:442Test relies on hardcoded column indexes:
    The assertions use cells[0] and cells[1] to identify File vs Model columns. This is brittle if columns are reordered, hidden, or responsive behavior changes, causing false failures.
    Suggestion: locate the target cells by column semantics instead of index (e.g., find header index by text columns.file / columns.model, then assert within that index; or query within row for model text container and ensure cloud icon is inside it).

Praise

  • src/renderer/pages/List.tsx:537Good UI alignment fix with flex container:
    Using display: flex + alignItems: center in the model cell is a clean way to keep text and icon aligned consistently.

  • src/renderer/pages/List.tsx:586Improved failure tooltip handling across task types:
    The fallback chain for error / error_message / description makes failure reasons more robust, especially when backend payloads differ.

  • src/renderer/utils/cloudTaskMapper.ts:65 and src/shared/types/cloud-api.ts:122Type contract and mapper updated together:
    Adding description and error_message in shared API types and mapping them through prevents unsafe access and keeps renderer logic type-safe.

@jorben jorben merged commit 9ae48af into master Mar 2, 2026
3 checks passed
@jorben jorben deleted the fix/cloud-icon-after-model branch March 2, 2026 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant