Skip to content

refactor(frontend): improve "go to actor" ux#4682

Merged
jog1t merged 1 commit intomainfrom
04-20-refactor_frontend_improve_go_to_actor_ux
Apr 21, 2026
Merged

refactor(frontend): improve "go to actor" ux#4682
jog1t merged 1 commit intomainfrom
04-20-refactor_frontend_improve_go_to_actor_ux

Conversation

@jog1t
Copy link
Copy Markdown
Contributor

@jog1t jog1t commented Apr 21, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app Bot commented Apr 21, 2026

🚅 Deployed to the rivet-pr-4682 environment in rivet-frontend

Service Status Web Updated (UTC)
mcp-hub ✅ Success (View Logs) Web Apr 21, 2026 at 2:50 pm
ladle ❌ Build Failed (View Logs) Web Apr 21, 2026 at 2:50 pm
kitchen-sink ❌ Build Failed (View Logs) Web Apr 21, 2026 at 2:50 pm
website 🕒 Building (View Logs) Web Apr 21, 2026 at 2:50 pm
frontend-inspector 🕒 Building (View Logs) Web Apr 21, 2026 at 2:49 pm
frontend-cloud 🕒 Building (View Logs) Web Apr 21, 2026 at 2:49 pm

@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-4682 April 21, 2026 14:49 Destroyed
@jog1t jog1t marked this pull request as ready for review April 21, 2026 14:49
Copy link
Copy Markdown
Contributor Author

jog1t commented Apr 21, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jog1t jog1t mentioned this pull request Apr 21, 2026
11 tasks
@jog1t jog1t requested a review from NathanFlurry April 21, 2026 14:50
Copy link
Copy Markdown
Contributor Author

jog1t commented Apr 21, 2026

Merge activity

  • Apr 21, 2:50 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 21, 2:50 PM UTC: @jog1t merged this pull request with Graphite.

@jog1t jog1t merged commit ebb012b into main Apr 21, 2026
22 of 32 checks passed
@jog1t jog1t deleted the 04-20-refactor_frontend_improve_go_to_actor_ux branch April 21, 2026 14:50
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 21, 2026

Code Review

Overall this is a solid UX improvement. The inline input replacing the modal dialog is a more direct interaction pattern. A few issues worth addressing before merging:


Bugs / Correctness

Double-submit on Enter + blur (go-to-actor-button.tsx)

When a user presses Enter, handleSubmit fires via handleKeyDown, and then the input loses focus immediately, triggering onBlur which calls handleSubmit a second time. Both calls start before either's finally block runs, so both navigate concurrently — this can result in the URL being mutated twice, potentially landing on the wrong actor.

Guard onBlur with isPending, or prevent the blur-triggered submit when Enter was already handled:

const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
    if (e.key === "Enter") {
        e.preventDefault(); // prevent blur-triggered submit
        void handleSubmit();
    }
    if (e.key === "Escape") {
        setValue("");
        setOpen(false);
    }
};

// then in JSX:
onBlur={() => { if (!isPending) void handleSubmit(); }}

CatchBoundary asymmetry between cloud and engine routes

The _engine route still wraps <Actors> in a CatchBoundary, but the cloud route (changed in this PR) no longer does. If the error boundary is intentionally removed, the engine route should match. If it's still needed, restore it here.

Silent fallback hides non-404 errors from the user

In handleSubmit, the catch block unconditionally navigates with actorKey: trimmed regardless of what the error is. A network error (e.g. 500, timeout) will redirect the user to a "not found" state instead of surfacing an actionable error. At minimum, errors that are clearly not "actor not found" should show a toast or inline error.


Unused Code (actors-list.tsx)

After Filters was simplified to just render <GoToActorButton />, the following are now dead code and should be removed:

  • Imports: faMagnifyingGlass, faBook, faExclamationTriangle, FilterCreator, H4
  • The entire useFiltersChangeCallback hook and its associated imports (useLocalStorage, useCallback, ls, type OnFiltersChange, pick/remove from useActorsFilters)

UX / Accessibility

No visible error feedback on failed lookup

When the actor ID doesn't resolve, the user is silently redirected with actorKey. There's no indication that "this ID was not found, treating as a key." A brief inline message or toast would clarify the behavior.

Unlabeled input

The <Input> rendered when open === true has a placeholder but no aria-label or associated <label>. Add aria-label="Go to Actor" for screen reader support.

ButtonProps not forwarded to the input state

GoToActorButton accepts props: ButtonProps and spreads them onto the button, but when open === true the component renders an <Input> that ignores props. Currently the component is called with no props, but the type signature implies they'd work — either document the limitation or forward relevant props to both render states.


Minor

Formatting-only changes in app.tsx are pure whitespace reformatting with no behavioral change — fine, but adds noise to the diff.

posthog import reordering in ns.$namespace.tsx looks correct and intentional.


Summary

Good direction. The main items to resolve: (1) double-submit race between Enter keydown and onBlur, (2) CatchBoundary asymmetry between routes, (3) non-404 errors being silently swallowed, and (4) removing orphaned imports and dead useFiltersChangeCallback code.

@github-actions
Copy link
Copy Markdown
Contributor

Preview packages published to npm

Install with:

npm install rivetkit@pr-4682

All packages published as 0.0.0-pr.4682.eca8092 with tag pr-4682.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-eca8092
docker pull rivetdev/engine:full-eca8092
Individual packages
npm install rivetkit@pr-4682
npm install @rivetkit/react@pr-4682
npm install @rivetkit/rivetkit-native@pr-4682
npm install @rivetkit/sqlite-wasm@pr-4682
npm install @rivetkit/workflow-engine@pr-4682

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