Skip to content

Change styling of position readout - #729

Merged
lawsie merged 2 commits into
flipcomputing:mainfrom
lawsie:pill-not-editable
Jul 30, 2026
Merged

Change styling of position readout#729
lawsie merged 2 commits into
flipcomputing:mainfrom
lawsie:pill-not-editable

Conversation

@lawsie

@lawsie lawsie commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

Change the styling of the position readout so that it does not appear editable when it isn't

image

AI usage

Claude Opus 5 used. Styling decided by me.

Summary by CodeRabbit

  • Style

    • Updated position readouts with a cleaner reading layout and colored axis indicators.
    • Replaced bordered “pill” styling with grouped, consistently sized numeric readings.
  • Bug Fixes

    • Position readouts now clear reliably when exiting gizmo mode.
    • Status handling remains consistent when switching between mesh and ground selections.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The position readout now renders grouped segments with axis color bars instead of bordered pills. Gizmo status entries explicitly use the position-readout owner and are cleared when gizmo state exits, with updated rendering and lifecycle tests.

Changes

Position readout

Layer / File(s) Summary
Grouped status rendering
ui/status.js, style.css, tests/status.test.js
Structured reading segments render recursively with axis color variables, while the old pill styling and assertions are removed.
Position readout ownership and cleanup
ui/gizmos.js, tests/gizmos.test.js
Position status updates use an explicit owner and barColor, and gizmo exit clears the readout with corresponding DOM coverage.

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

Possibly related PRs

  • flipcomputing/flock#713: Introduced the status element and baseline status styling related to the updated readout renderer.

Suggested reviewers: tracygardner

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: the position readout styling was updated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/gizmos.test.js (1)

258-270: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Exercise the actual gizmo-switch path.

This test calls exitGizmoState() directly, so it does not prove that selecting a different gizmo clears the readout. Switch between two gizmos through the public selection path and assert the position-owned status is cleared.

🤖 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 `@tests/gizmos.test.js` around lines 258 - 270, Update the test around
exitGizmoState to exercise the public gizmo-selection path: select one gizmo,
set the position-owned status, then select a different gizmo and assert the
status text is cleared. Do not call exitGizmoState directly; use the existing
public gizmo selection symbol and preserve the cleanup logic.
🤖 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 `@tests/gizmos.test.js`:
- Around line 258-270: Update the test around exitGizmoState to exercise the
public gizmo-selection path: select one gizmo, set the position-owned status,
then select a different gizmo and assert the status text is cleared. Do not call
exitGizmoState directly; use the existing public gizmo selection symbol and
preserve the cleanup logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 20ab1cbe-8afb-4057-909c-b99aba2b7e44

📥 Commits

Reviewing files that changed from the base of the PR and between 998e194 and fdb116f.

📒 Files selected for processing (5)
  • style.css
  • tests/gizmos.test.js
  • tests/status.test.js
  • ui/gizmos.js
  • ui/status.js

@lawsie
lawsie merged commit f8629a3 into flipcomputing:main Jul 30, 2026
3 checks passed
@lawsie
lawsie deleted the pill-not-editable branch July 30, 2026 09:10
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