Skip to content

Major improvemts on UX#11

Merged
ripventura merged 5 commits intomainfrom
feat/immersive-view
Feb 19, 2026
Merged

Major improvemts on UX#11
ripventura merged 5 commits intomainfrom
feat/immersive-view

Conversation

@ripventura
Copy link
Copy Markdown
Owner

Added support for an immersive environment where the user can view the asset in full scale. Immersive environment can be entered by touching the buttons at the UI level or turning the digital crown (impossible on the simulator).

Added support for an immersive environment where the user can view the asset in full scale. Immersive environment can be entered by touching the buttons at the UI level or turning the digital crown (impossible on the simulator).
@ripventura ripventura self-assigned this Feb 19, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Feb 19, 2026

Code review

Bug: Re-entrant openImmersiveSpace calls during .opening phaseMuseum/Views/AssetDetailView.swift, line 40

The "View Immersive" button is shown whenever hasLoadedModel == true, with no check on immersiveSpaceController.phase. While openImmersiveSpace is in-flight (phase == .opening), the button stays visible and tappable, allowing a second concurrent call to openImmersiveSpace before the first resolves — an API contract violation on visionOS.

.ornament(attachmentAnchor: .scene(.bottomFront)) {
if hasLoadedModel {
Button("View Immersive") {
openImmersive()
}

Fix: gate the button condition on phase too:

if hasLoadedModel && immersiveSpaceController.phase == .closed {

@ripventura
Copy link
Copy Markdown
Owner Author

@claude Apply the fix suggested on this review: #11 (comment)

@claude
Copy link
Copy Markdown

claude Bot commented Feb 19, 2026

Claude finished @ripventura's task in 41s —— View job


Fix applied: re-entrant openImmersiveSpace guard

  • Read AssetDetailView.swift to understand the current code
  • Apply the fix: gate the "View Immersive" button on phase == .closed
  • Commit and push the change

The "View Immersive" button in AssetDetailView.swift:40 now checks both hasLoadedModel and immersiveSpaceController.phase == .closed before rendering:

// Before
if hasLoadedModel {

// After
if hasLoadedModel && immersiveSpaceController.phase == .closed {

This prevents concurrent openImmersiveSpace calls — the button is hidden while the phase is .opening or .open, so it can only be tapped when the immersive space is fully closed.
·

github-actions Bot and others added 2 commits February 19, 2026 22:22
claude[bot]

This comment was marked as outdated.

@claude
Copy link
Copy Markdown

claude Bot commented Feb 19, 2026

Code review

Bug: .error phase is a permanent dead-end — user gets stuck on spinner with no recovery

immersiveSpaceController.phase = .closed
case .error:
immersiveSpaceController.phase = .error
@unknown default:
immersiveSpaceController.phase = .error
}

When openImmersiveSpace returns .error, phase is set to .error. The ornament condition hasLoadedModel && phase == .closed is never true again because nothing transitions phase back to .closedImmersiveAssetView.onDisappear is the only reset path, but it never fires since the space never opened. The else branch renders the "Loading experience..." spinner forever with no way to retry.

Fix by treating .error the same as .userCancelled:

case .error:
    immersiveSpaceController.phase = .closed
@unknown default:
    immersiveSpaceController.phase = .closed

@ripventura
Copy link
Copy Markdown
Owner Author

@claude Fix this issue #11 (comment)

@claude
Copy link
Copy Markdown

claude Bot commented Feb 19, 2026

Claude finished @ripventura's task in 48s —— View job


Fix applied: .error phase dead-end

  • Understand the request
  • Read AssetDetailView.swift
  • Apply fix: reset phase to .closed on .error and @unknown default
  • Commit and push

Both case .error and @unknown default in the openImmersive() function (AssetDetailView.swift:70-73) now reset phase to .closed instead of .error:

// Before
case .error:
    immersiveSpaceController.phase = .error
@unknown default:
    immersiveSpaceController.phase = .error

// After
case .error:
    immersiveSpaceController.phase = .closed
@unknown default:
    immersiveSpaceController.phase = .closed

When openImmersiveSpace fails, the phase returns to .closed, which makes the "View Immersive" button reappear in the ornament — giving the user a retry path instead of being stuck on the spinner indefinitely.

github-actions Bot and others added 2 commits February 19, 2026 22:43
…ce failure

When openImmersiveSpace returns .error or an unknown result, the phase is now
reset to .closed instead of .error. This allows the "View Immersive" button to
reappear so the user can retry, avoiding a permanent spinner with no recovery.

Co-authored-by: Vitor Cesco <ripventura@users.noreply.github.com>
@ripventura ripventura merged commit 73efa1d into main Feb 19, 2026
1 check passed
@ripventura ripventura deleted the feat/immersive-view branch February 19, 2026 22:49
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