Skip to content

Fix inverted play/pause glyphs on the site status toggle and add a status tooltip - #4374

Merged
shaunandrews merged 3 commits into
trunkfrom
stu-2153-fix-inverted-play-pause-icons
Jul 28, 2026
Merged

Fix inverted play/pause glyphs on the site status toggle and add a status tooltip#4374
shaunandrews merged 3 commits into
trunkfrom
stu-2153-fix-inverted-play-pause-icons

Conversation

@shaunandrews

@shaunandrews shaunandrews commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related issues

How AI was used in this PR

Claude located the inverted ternary, confirmed the glyph semantics against the CSS module (.pauseIcon renders two bars, .playIcon renders a triangle), and applied the one-line swap. It then mirrored the sidebar's SiteStatusButton tooltip pattern onto the dropdown toggle and added the test coverage. Reviewed by me.

Proposed Changes

image image

The icons for the play/pause toggle in the Site Menu were reversed; you clicked play to stop, and pause to start. The PR changes that so you click pause to stop, and play to start. I also added a tooltip, and generally matched the behavior of the toggle buttons in the sidebar.

Testing Instructions

  1. Run the agentic UI and open the site dropdown for a stopped site.
  2. The status toggle should show a play triangle, and hovering it should read "Site status: Stopped". Click it — the site starts.
  3. While it starts, the tooltip should read "Site status: Starting" and the glyph should already show pause (the target state). Clicking during this window should do nothing.
  4. Once running, the toggle should show pause bars and read "Site status: Running". Click it — the site stops, and the tooltip reads "Site status: Stopping" on the way down.
  5. Start a sync (push or pull) and confirm the toggle dims, still shows its tooltip, and ignores clicks.
  6. Confirm the dimmed/inert styling still looks right — the CSS moved from :disabled to [aria-disabled='true'].
  7. Confirm all of the above in both light and dark mode.
@shaunandrews @claude
Fix inverted play/pause glyphs on the site status toggle
62e0d0b 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shaunandrews @claude
Add a status tooltip to the site status toggle, matching the sidebar
6c5a84b 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shaunandrews shaunandrews changed the title Fix inverted play/pause glyphs on the site status toggle Jul 28, 2026
@shaunandrews
shaunandrews requested review from a team and wojtekn July 28, 2026 13:55
@wpmobilebot

wpmobilebot commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing a681916 vs trunk

app-size

Metric trunk a681916 Diff Change
App Size (Mac) 1375.36 MB 1375.36 MB +0.00 MB ⚪ 0.0%

site-editor

Metric trunk a681916 Diff Change
load 1079 ms 1074 ms 5 ms ⚪ 0.0%

site-startup

Metric trunk a681916 Diff Change
siteCreation 6485 ms 6646 ms +161 ms 🔴 2.5%
siteStartup 2384 ms 2381 ms 3 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)
wojtekn
wojtekn approved these changes Jul 28, 2026
View reviewed changes

@wojtekn wojtekn 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.

Looks good, works as expected. Left one comment regarding code readability.
Comment thread apps/ui/src/components/site-dropdown/main-view.tsx Outdated
// the pointer events the tooltip listens for, hiding the status exactly
// while the site is transitioning.
const inert = disabled || pending;
const statusName = pending

@wojtekn wojtekn Jul 28, 2026

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.

Chained ternary is not really readable, I would split it if possible.

@shaunandrews shaunandrews Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call — split into a getLocalServerStatusName() helper with early returns in a681916.
@shaunandrews @claude
Extract the status name into a helper instead of a chained ternary
a681916 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shaunandrews
shaunandrews merged commit 9055fee into trunk Jul 28, 2026
15 checks passed
@shaunandrews
shaunandrews deleted the stu-2153-fix-inverted-play-pause-icons branch July 28, 2026 23:41
@shaunandrews shaunandrews mentioned this pull request Jul 29, 2026
Merged
1 task
shaunandrews added a commit that referenced this pull request Jul 29, 2026
@shaunandrews @claude
Fix site status toggle tests to use the current renderMainView signat…
427be4f 
…ure (#4379)

## Related issues

- Related to #4374 and #4307.

## How AI was used in this PR

AI traced the failure to its origin, wrote the fix, and verified it.
`npm run typecheck` and the affected suite were run locally and
reviewed.

## Proposed Changes

`trunk` is currently red on both **Unit Tests** and **Lint**. This fixes
it.

Two PRs landed the same day and conflicted semantically rather than
textually, so nothing blocked either one:

- #4307 changed the `renderMainView` test helper to take an options
object (`{ siteOverrides, activity }`) instead of a bare site-overrides
argument, updating the call sites that existed at the time.
- #4374 was branched before that and added two calls using the previous
signature, then merged about seven hours later.

Git merged both cleanly — the two PRs touched different lines of the
same file — but the calls no longer match the helper. `{ running: false
}` is now swallowed as the options object, `siteOverrides` falls back to
`{}`, and the site under test stays *running*. One test then looks for a
stopped-site toggle that was never rendered.

The second call site is worth noting: it fails typecheck but still
passes, because that test sets `starting` and the pending label wins
regardless of `running`. It was quietly asserting against a running site
while reading as though it covered a stopped one. Both call sites now
pass the override through properly, so that coverage is real again.

No production code changes — this is test-only, and the behavior #4374
shipped is unaffected.

## Testing Instructions

1. `npm test -- apps/ui/src/components/site-dropdown/main-view.test.tsx`
— 9 passing.
2. `npm run typecheck` — clean; on `trunk` it reports two `TS2353`
errors in this file.
3. Confirm CI is green on both Unit Tests and Lint (the Lint job runs
`npm run lint` followed by `npm run typecheck`, which is why one root
cause turned two checks red).

## Pre-merge Checklist

- [x] Have you checked for TypeScript, React or other console errors?

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants

@shaunandrews @wpmobilebot @wojtekn