feat(ui): add scalable instance library workspace - #181
Merged
Merged
Conversation
Contributor
Workspace change through: 741e4700 changesets found Planned changes to release
|
Contributor
Reviewer's GuideRedesigns the Instances page into a scalable "instance library" with search, sorting, list/grid layouts, readiness probes, and an active detail workspace, while wiring it into refreshed instance-store behavior, fixtures, tests, and documentation for both English and Chinese. Sequence diagram for the new instance library launch flowsequenceDiagram
actor User
participant InstancesPage
participant InstanceStore as useInstanceStore
participant GameStore as useGameStore
participant Client as ClientAPI
User->>InstancesPage: click LibraryItem.onLaunch(instance)
activate InstancesPage
InstancesPage->>InstancesPage: launchInstance(instance)
note over InstancesPage: Ensure instance is active
InstancesPage->>InstanceStore: setActiveInstance(instance)
activate InstanceStore
InstanceStore-->>InstancesPage: Promise resolved
deactivate InstanceStore
InstancesPage->>InstancesPage: checkReadiness(instance)
InstancesPage->>Client: getLaunchReadiness(instance.id, instance.versionId)
activate Client
Client-->>InstancesPage: LaunchReadiness
deactivate Client
InstancesPage->>InstancesPage: setReadinessById({ status: ready/attention })
alt runningInstanceId === instance.id
InstancesPage->>GameStore: stopGame(instance.id)
activate GameStore
GameStore-->>InstancesPage: stop complete
deactivate GameStore
else not running
alt !instance.versionId
InstancesPage->>InstancesPage: toast.error("instances.noVersionError")
else !account
InstancesPage->>InstancesPage: toast.info("instances.loginFirst")
else ready to launch
InstancesPage->>GameStore: startGame(instance.id, instance.versionId)
activate GameStore
GameStore-->>InstancesPage: launch started
deactivate GameStore
end
end
deactivate InstancesPage
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/ui/src/pages/instances/index.tsx" line_range="702-710" />
<code_context>
+ const activeInstance = useInstanceStore((state) => state.activeInstance);
+ const instanceStatus = useInstanceStore((state) => state.status);
+ const instanceError = useInstanceStore((state) => state.error);
+ const refreshInstances = useInstanceStore((state) => state.refresh);
+ const setActiveInstance = useInstanceStore(
+ (state) => state.setActiveInstance,
+ );
+ const deleteInstance = useInstanceStore((state) => state.delete);
+ const duplicateInstance = useInstanceStore((state) => state.duplicate);
+ const exportArchive = useInstanceStore((state) => state.exportArchive);
+ const importArchive = useInstanceStore((state) => state.importArchive);
+ const repairInstances = useInstanceStore((state) => state.repair);
const account = useAuthStore((state) => state.account);
+ const accountIdentity = account
</code_context>
<issue_to_address>
**issue (bug_risk):** Instances are never initially refreshed, so the library can stay empty with status "idle"
In the previous version, `instancesStore.refresh()` was called on mount via `useEffect`; the new code never triggers `refreshInstances()`. This means `instances` and `instanceStatus` stay at their initial values, so the library can appear empty with status `idle` even when instances exist. Please add an initial `useEffect` (and, if needed, react to fixture changes) to call `refreshInstances()` to preserve the prior behavior.
</issue_to_address>
### Comment 2
<location path="packages/ui/src/pages/instances/index.tsx" line_range="315-324" />
<code_context>
+ disabled={saving}
+ className="flex-1"
+ />
+ <Button
+ type="button"
+ variant="outline"
</code_context>
<issue_to_address>
**nitpick:** Export buttons don’t reflect the exporting state consistently in the detail workspace
In the library item dropdown the export button reflects `exporting` via its label, but in `DetailWorkspace` the shortcut button only disables without changing the label. For long-running exports this leads to inconsistent feedback depending on where the user triggers export. Please reuse the same label logic (or add a spinner) in `DetailWorkspace` to make the export state clear and consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+702
to
+710
| const refreshInstances = useInstanceStore((state) => state.refresh); | ||
| const setActiveInstance = useInstanceStore( | ||
| (state) => state.setActiveInstance, | ||
| ); | ||
| const deleteInstance = useInstanceStore((state) => state.delete); | ||
| const duplicateInstance = useInstanceStore((state) => state.duplicate); | ||
| const exportArchive = useInstanceStore((state) => state.exportArchive); | ||
| const importArchive = useInstanceStore((state) => state.importArchive); | ||
| const repairInstances = useInstanceStore((state) => state.repair); |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Instances are never initially refreshed, so the library can stay empty with status "idle"
In the previous version, instancesStore.refresh() was called on mount via useEffect; the new code never triggers refreshInstances(). This means instances and instanceStatus stay at their initial values, so the library can appear empty with status idle even when instances exist. Please add an initial useEffect (and, if needed, react to fixture changes) to call refreshInstances() to preserve the prior behavior.
Comment on lines
+315
to
+324
| <Button | ||
| type="button" | ||
| size={view === "grid" ? "sm" : "icon"} | ||
| variant={running ? "destructive" : active ? "default" : "outline"} | ||
| aria-label={launchLabel} | ||
| title={launchLabel} | ||
| className={cn(view === "grid" && "flex-1")} | ||
| onClick={() => onLaunch(instance)} | ||
| disabled={anotherRunning || launching || stopping} | ||
| > |
Contributor
There was a problem hiding this comment.
nitpick: Export buttons don’t reflect the exporting state consistently in the detail workspace
In the library item dropdown the export button reflects exporting via its label, but in DetailWorkspace the shortcut button only disables without changing the label. For long-running exports this leads to inconsistent feedback depending on where the user triggers export. Please reuse the same label logic (or add a spinner) in DetailWorkspace to make the export state clear and consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Impact
Large instance collections remain scannable at the default and minimum launcher sizes, while launch remains the prominent action and destructive actions retain explicit recovery guidance.
Validation
pnpm -C packages/ui lintpnpm -C packages/ui buildpnpm docs:buildpnpm -C packages/ui test:ui— 102 passed on macOSDisclosure
This PR contains LLM-assisted code with author-provided quality assurance.
Closes #175
Summary by Sourcery
Redesign the instances page into a searchable, sortable instance library with a persistent list/grid view and an active detail workspace that stays synchronized with the launcher.
New Features:
Enhancements:
Documentation:
Tests: