fix(frontend): polish actors list ui#5120
Conversation
…gets, timestamp size)
|
🚅 Deployed to the rivet-pr-5120 environment in rivet-frontend
|
NicholasKissel
changed the title
Code Review:
|
| Area | Change | Assessment |
|---|---|---|
| Display + Search buttons | outline → ghost variant |
Correct — matches the create button's ghost style |
| Timestamp | Adds explicit text-xs override |
Works, but see note below |
| Status column header | <div>Status</div> → <div /> |
Clean; the colored dot is self-explanatory |
| ID / Key copy area | Adds justify-self-start |
Correct grid property to shrink the hit area |
Suggestions / observations
1. text-xs override on SmallText is a smell
// actors-list-row.tsx:183
<SmallText className="text-right text-xs text-muted-foreground justify-end inline-flex tabular-nums">The explicit text-xs is needed to override whatever size SmallText bakes in. If every caller in this context wants text-xs, the component itself should be updated (or a narrower variant introduced) rather than having each call-site fight it. Not a blocker, but worth a follow-up to avoid the pattern spreading.
2. <div /> self-closing form
// actors-list-row.tsx:208
<div />This is valid JSX, but some codebases prefer <div></div> for block elements to make it visually clear it's a block container. Check neighboring code — if the convention is <div></div>, keep it consistent.
3. justify-self-start on an inline-flex element
<span className="... inline-flex ... justify-self-start ...">justify-self is a grid item property (not a flex property), so it only takes effect because the parent is a grid container. This is fine and correct for the current structure, but it's worth noting that removing the parent's grid or reparenting these spans could silently break the alignment. No action required unless the layout changes.
No concerns on
- Security: purely presentational, no data flows changed.
- Performance: no new re-renders or expensive operations introduced.
- Test coverage: visual polish changes are generally not unit-tested; the PR description clearly documents before/after intent.
Summary
The changes are correct, minimal, and well-motivated. The only real note worth acting on is the text-xs override on SmallText — if that component always renders too large in tabular contexts, updating the component itself would be cleaner than per-call-site overrides. Everything else looks good to merge.
1 participant
What?
Small visual polish to the Actors instance list.
ghostvariant so they match the muted+(create) button instead of rendering as dark borderedoutlinebuttons.text-smtotext-xsso it matches the rest of the row text (SmallTextwas forcing the larger size).justify-self-start, instead of stretching across the whole grid cell. Long values still truncate within the column.Why?
These are follow-up UI tweaks on the actors list flagged during review: the Display/search controls looked inconsistent with the create button, the timestamp was visually oversized, the STATUS header was redundant, and the full-width copy hit area made it easy to copy a key by accident when clicking anywhere in the row.