Skip to content

[companion] feat(settings): show attention indicator on tabs and update cards - #32

Open
andrebrait wants to merge 4 commits into
companion/upstream-mainfrom
feat/settings-update-attention
Open

andrebrait wants to merge 4 commits into
companion/upstream-mainfrom
feat/settings-update-attention

Conversation

@andrebrait

@andrebrait andrebrait commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Companion PR for kahme247#115 to run bot reviews (Copilot, CodeRabbit).

Upstream PR: kahme247#115

Summary by CodeRabbit

  • New Features

    • Settings now highlight available app and OMP updates with accent styling and status indicators.
    • The System tab displays an attention badge when updates require review.
    • Added accessible update labels and visual indicators across vertical and horizontal settings layouts.
    • Added English, Japanese, and Simplified Chinese translations for update notifications.
  • Tests

    • Added coverage for settings tab attention indicators and accessible labels.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c3ef93ef-9c24-4ab3-98ec-387bb426090f

📥 Commits

Reviewing files that changed from the base of the PR and between 330db7d and f902c7c.

📒 Files selected for processing (7)
  • components/AppShell.tsx
  • components/SettingsConfig.tsx
  • components/SettingsTabs.test.mjs
  • components/SettingsTabs.tsx
  • lib/i18n/locales/en.json
  • lib/i18n/locales/ja.json
  • lib/i18n/locales/zh-CN.json

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

AppShell passes OMP update state to SettingsConfig. Settings derives update indicators for the system tab and update cards. SettingsTabs renders attention indicators, accessible labels, and localized messages in both layouts.

Changes

Settings update attention

Layer / File(s) Summary
Update state and card indicators
components/AppShell.tsx, components/SettingsConfig.tsx
SettingsConfig receives OMP update availability, derives update flags, marks the System tab, and highlights app and OMP update cards.
Settings tab attention rendering
components/SettingsTabs.tsx, components/SettingsTabs.test.mjs, lib/i18n/locales/*.json
Settings tabs show attention dots and accessible update text in vertical and horizontal layouts. English, Japanese, and Chinese labels are included, with tests for the rendered labels.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant AppShell
  participant SettingsConfig
  participant SettingsTabs
  AppShell->>SettingsConfig: pass ompUpdateAvailable
  SettingsConfig->>SettingsConfig: derive update flags
  SettingsConfig->>SettingsTabs: pass attentionTabs
  SettingsTabs->>SettingsTabs: render indicator and accessible label
Loading

Suggested reviewers: kahme247

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main changes: attention indicators on settings tabs and update cards.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@andrebrait
andrebrait requested a lite review from Copilot September 19, 2026 08:06
@andrebrait

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Pull request base or head changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Runtime attention state can become stale, failed checks can show an indefinite loading message, and vertical tab indicators are not reliably accessible.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 Medium severity

Open (3)
What changed in this PR

Adds update-attention indicators to Settings tabs and update cards, with localized labels and rendering tests.

Changes:

  • Added localized attention strings.
  • Propagated runtime update state into settings and highlighted update cards.
  • Added vertical/horizontal tab indicator tests.
File Description
components/​AppShell.tsx Passes runtime update availability to settings.
components/​SettingsConfig.tsx Displays attention indicators and highlighted update cards.
components/​SettingsTabs.tsx Renders attention indicators for settings tabs.
components/​SettingsTabs.test.mjs Tests attention indicators in both layouts.
lib/​i18n/​locales/​en.json Adds English attention labels.
lib/​i18n/​locales/​ja.json Adds Japanese attention labels.
lib/​i18n/​locales/​zh-CN.json Adds Chinese attention labels.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread components/SettingsConfig.tsx Outdated
Comment thread components/SettingsConfig.tsx Outdated
Comment thread components/SettingsTabs.tsx Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

OMP update styling can become inconsistent because the new status color uses nullish coalescing instead of combining both availability sources.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
Resolved since last review (3)

Comment thread components/SettingsConfig.tsx Outdated
@andrebrait

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Head commit changed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@andrebrait
andrebrait requested a lite review from Copilot September 19, 2026 08:20
@andrebrait

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The changes are localized, tested across both layouts, and introduce no unresolved correctness issues.

Review effort: Lite
Findings: None

Resolved since last review (1)

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.

2 participants