mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(mobile): load markdown tabs from any desktop split group; center load-error Retry (#28132)
* fix(mobile): load markdown tabs from any desktop split group and center load-error Retry The desktop answered markdown.readTab only for tabs in its focused split group, so a phone opening a markdown tab from another group got tab_not_found and showed "Couldn't load markdown". Resolve the tab across all of the worktree's groups. Load-error states now share one component that centers the message and its Retry button; the markdown reader's button was pinned to the left. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(renderer): mobile markdown read picks the requested tab across groups Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(renderer): resolve mobile markdown tab to one file id Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
@@ -0,0 +1,51 @@
|
||||
import { Pressable, StyleSheet, Text, View } from 'react-native'
|
||||
import { colors, radii, spacing, typography } from '../theme/mobile-theme'
|
||||
|
||||
/** A failed load: the message with its Retry button centered under it as one group. */
|
||||
export function MobileLoadErrorState({
|
||||
message,
|
||||
onRetry
|
||||
}: {
|
||||
message: string
|
||||
onRetry?: (() => void) | null
|
||||
}) {
|
||||
return (
|
||||
<View style={styles.state}>
|
||||
<Text style={styles.errorText}>{message}</Text>
|
||||
{onRetry ? (
|
||||
<Pressable style={styles.retryButton} onPress={onRetry} accessibilityRole="button">
|
||||
<Text style={styles.retryText}>Retry</Text>
|
||||
</Pressable>
|
||||
) : null}
|
||||
</View>
|
||||
)
|
||||
}
|
||||
|
||||
const styles = StyleSheet.create({
|
||||
state: {
|
||||
flex: 1,
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center',
|
||||
gap: spacing.md,
|
||||
padding: spacing.xl
|
||||
},
|
||||
errorText: {
|
||||
color: colors.statusRed,
|
||||
fontSize: typography.bodySize,
|
||||
textAlign: 'center'
|
||||
},
|
||||
retryButton: {
|
||||
minHeight: 36,
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center',
|
||||
borderRadius: radii.button,
|
||||
borderWidth: StyleSheet.hairlineWidth,
|
||||
borderColor: colors.borderSubtle,
|
||||
paddingHorizontal: spacing.lg
|
||||
},
|
||||
retryText: {
|
||||
color: colors.textPrimary,
|
||||
fontSize: typography.bodySize,
|
||||
fontWeight: '600'
|
||||
}
|
||||
})
|
||||
@@ -34,6 +34,7 @@ import { fileDirectoryRead, legacyFileListRead } from './mobile-file-explorer-op
|
||||
import { fileExplorerStyles as styles } from './mobile-file-explorer-styles'
|
||||
import { MobileFileExplorerRow } from './mobile-file-explorer-row'
|
||||
import { navigateToMobileFilePreview } from './mobile-file-preview-navigation'
|
||||
import { MobileLoadErrorState } from '../components/MobileLoadErrorState'
|
||||
|
||||
export function MobileFileExplorerPanel(props: {
|
||||
hostId: string
|
||||
@@ -337,14 +338,7 @@ export function MobileFileExplorerPanel(props: {
|
||||
<ActivityIndicator size="small" color={colors.textSecondary} />
|
||||
</View>
|
||||
) : error ? (
|
||||
<View style={styles.state}>
|
||||
<Text style={styles.errorText}>{error}</Text>
|
||||
{rootRetry ? (
|
||||
<Pressable style={styles.retryButton} onPress={rootRetry}>
|
||||
<Text style={styles.retryText}>Retry</Text>
|
||||
</Pressable>
|
||||
) : null}
|
||||
</View>
|
||||
<MobileLoadErrorState message={error} onRetry={rootRetry} />
|
||||
) : rows.length === 0 ? (
|
||||
<View style={styles.state}>
|
||||
<Text style={styles.emptyText}>No files found</Text>
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useEffect, useState } from 'react'
|
||||
import { ActivityIndicator, Pressable, Text, View } from 'react-native'
|
||||
import { ActivityIndicator, Text, View } from 'react-native'
|
||||
import { colors } from '../theme/mobile-theme'
|
||||
import type { MobileFilePreviewRpcSender } from './mobile-file-preview-operations'
|
||||
import type { MobileFileMedia } from './mobile-file-media'
|
||||
@@ -8,6 +8,7 @@ import { createMobileMediaSink } from './mobile-media-preview-cache'
|
||||
import { MobileMediaPlayback } from './MobileMediaPlayback'
|
||||
import { filePreviewStyles as styles } from './mobile-file-preview-styles'
|
||||
import { formatPreviewByteLength } from './mobile-file-preview-response'
|
||||
import { MobileLoadErrorState } from '../components/MobileLoadErrorState'
|
||||
|
||||
type State = { uri: string } | { error: string } | { bytes: number; total: number }
|
||||
type Props = { media: MobileFileMedia; client: MobileFilePreviewRpcSender | null; title: string }
|
||||
@@ -62,18 +63,7 @@ function MediaDownload({ media, client, title }: Props) {
|
||||
return <MobileMediaPlayback key={state.uri} uri={state.uri} mimeType={mimeType} title={title} />
|
||||
}
|
||||
if ('error' in state) {
|
||||
return (
|
||||
<View style={styles.state}>
|
||||
<Text style={styles.errorText}>{state.error}</Text>
|
||||
<Pressable
|
||||
style={styles.retryButton}
|
||||
onPress={() => setAttempt(attempt + 1)}
|
||||
accessibilityRole="button"
|
||||
>
|
||||
<Text style={styles.retryText}>Retry</Text>
|
||||
</Pressable>
|
||||
</View>
|
||||
)
|
||||
return <MobileLoadErrorState message={state.error} onRetry={() => setAttempt(attempt + 1)} />
|
||||
}
|
||||
return (
|
||||
<View style={styles.state}>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { MobileFileMediaPreview } from './MobileFileMediaPreview'
|
||||
import type { MobileFilePreviewRpcSender } from './mobile-file-preview-operations'
|
||||
import { ActivityIndicator, Image, Pressable, ScrollView, Text, View } from 'react-native'
|
||||
import { ActivityIndicator, Image, ScrollView, Text, View } from 'react-native'
|
||||
import { colors } from '../theme/mobile-theme'
|
||||
import type { MobileFilePreviewResult } from './mobile-file-preview-request'
|
||||
import { MobileFileMarkdownPreview } from './MobileFileMarkdownPreview'
|
||||
@@ -8,6 +8,7 @@ import { MobileFilePreviewEditableSource } from './MobileFilePreviewEditableSour
|
||||
import { MobileFilePreviewSourceText } from './MobileFilePreviewSourceText'
|
||||
import type { MobileFilePreviewLineColumn } from './mobile-file-preview-line-column'
|
||||
import { filePreviewStyles as styles } from './mobile-file-preview-styles'
|
||||
import { MobileLoadErrorState } from '../components/MobileLoadErrorState'
|
||||
|
||||
type Props = {
|
||||
preview: MobileFilePreviewResult
|
||||
@@ -36,16 +37,7 @@ export function MobileFilePreviewBody({ preview, ...options }: Props) {
|
||||
)
|
||||
}
|
||||
if (preview.status === 'error' || preview.status === 'waiting') {
|
||||
return (
|
||||
<View style={styles.state}>
|
||||
<Text style={styles.errorText}>{preview.message}</Text>
|
||||
{options.onRetry ? (
|
||||
<Pressable style={styles.retryButton} onPress={options.onRetry}>
|
||||
<Text style={styles.retryText}>Retry</Text>
|
||||
</Pressable>
|
||||
) : null}
|
||||
</View>
|
||||
)
|
||||
return <MobileLoadErrorState message={preview.message} onRetry={options.onRetry} />
|
||||
}
|
||||
if (preview.status === 'empty') {
|
||||
return options.editable ? (
|
||||
|
||||
@@ -119,24 +119,5 @@ export const fileExplorerStyles = StyleSheet.create({
|
||||
emptyText: {
|
||||
color: colors.textSecondary,
|
||||
fontSize: typography.bodySize
|
||||
},
|
||||
errorText: {
|
||||
color: colors.statusRed,
|
||||
fontSize: typography.bodySize,
|
||||
textAlign: 'center'
|
||||
},
|
||||
retryButton: {
|
||||
minHeight: 36,
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center',
|
||||
borderRadius: radii.button,
|
||||
borderWidth: StyleSheet.hairlineWidth,
|
||||
borderColor: colors.borderSubtle,
|
||||
paddingHorizontal: spacing.lg
|
||||
},
|
||||
retryText: {
|
||||
color: colors.textPrimary,
|
||||
fontSize: typography.bodySize,
|
||||
fontWeight: '600'
|
||||
}
|
||||
})
|
||||
|
||||
@@ -59,20 +59,6 @@ export const filePreviewStyles = StyleSheet.create({
|
||||
fontSize: typography.bodySize,
|
||||
textAlign: 'center'
|
||||
},
|
||||
retryButton: {
|
||||
minHeight: 36,
|
||||
alignItems: 'center',
|
||||
justifyContent: 'center',
|
||||
borderRadius: radii.button,
|
||||
borderWidth: StyleSheet.hairlineWidth,
|
||||
borderColor: colors.borderSubtle,
|
||||
paddingHorizontal: spacing.lg
|
||||
},
|
||||
retryText: {
|
||||
color: colors.textPrimary,
|
||||
fontSize: typography.bodySize,
|
||||
fontWeight: '600'
|
||||
},
|
||||
saveButton: {
|
||||
width: 36,
|
||||
height: 36,
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
} from './markdown-floating-actions-layout'
|
||||
import type { MarkdownDocState } from './mobile-session-route-types'
|
||||
import { styles } from './mobile-session-styles'
|
||||
import { MobileLoadErrorState } from '../components/MobileLoadErrorState'
|
||||
|
||||
type Props = {
|
||||
documentId: string
|
||||
@@ -48,15 +49,7 @@ export function MobileMarkdownReader({
|
||||
)
|
||||
}
|
||||
if (doc.status === 'error') {
|
||||
return (
|
||||
<View style={styles.markdownState}>
|
||||
<Text style={styles.markdownError}>{doc.message}</Text>
|
||||
<Pressable style={styles.markdownRefreshButton} onPress={onRefresh}>
|
||||
<RefreshCw size={14} color={colors.textPrimary} />
|
||||
<Text style={styles.markdownRefreshText}>Retry</Text>
|
||||
</Pressable>
|
||||
</View>
|
||||
)
|
||||
return <MobileLoadErrorState message={doc.message} onRetry={onRefresh} />
|
||||
}
|
||||
|
||||
const statusText = doc.saveError
|
||||
|
||||
@@ -33,6 +33,7 @@ import type {
|
||||
} from './mobile-session-route-types'
|
||||
import { DiffLineRow } from './MobileSessionDiffLineRow'
|
||||
import { sendableMobileDiffComments } from './mobile-diff-comments'
|
||||
import { MobileLoadErrorState } from '../components/MobileLoadErrorState'
|
||||
|
||||
export function FileReader({
|
||||
doc,
|
||||
@@ -181,11 +182,7 @@ export function FileReader({
|
||||
)
|
||||
}
|
||||
if (doc.status === 'error') {
|
||||
return (
|
||||
<View style={styles.markdownState}>
|
||||
<Text style={styles.markdownError}>{doc.message}</Text>
|
||||
</View>
|
||||
)
|
||||
return <MobileLoadErrorState message={doc.message} />
|
||||
}
|
||||
|
||||
if (doc.kind === 'diff') {
|
||||
|
||||
@@ -8,7 +8,8 @@ vi.mock('react-native', () => ({
|
||||
View: 'View',
|
||||
Text: 'Text',
|
||||
Pressable: 'Pressable',
|
||||
ActivityIndicator: 'ActivityIndicator'
|
||||
ActivityIndicator: 'ActivityIndicator',
|
||||
StyleSheet: { create: <T,>(styles: T) => styles, hairlineWidth: 1 }
|
||||
}))
|
||||
vi.mock('lucide-react-native', () => ({ RefreshCw: 'RefreshCw' }))
|
||||
vi.mock('../components/MobileRichMarkdownEditor', () => ({
|
||||
|
||||
@@ -7,6 +7,7 @@ import { markdownReaderStatusText } from './mobile-markdown-reader-status'
|
||||
import { colors, spacing } from '../theme/mobile-theme'
|
||||
import { styles } from './mobile-session-styles'
|
||||
import type { MarkdownDocState } from './mobile-session-route-types'
|
||||
import { MobileLoadErrorState } from '../components/MobileLoadErrorState'
|
||||
|
||||
export function MarkdownReader({
|
||||
documentId,
|
||||
@@ -38,15 +39,7 @@ export function MarkdownReader({
|
||||
)
|
||||
}
|
||||
if (doc.status === 'error') {
|
||||
return (
|
||||
<View style={styles.markdownState}>
|
||||
<Text style={styles.markdownError}>{doc.message}</Text>
|
||||
<Pressable style={styles.markdownRefreshButton} onPress={onRefresh}>
|
||||
<RefreshCw size={14} color={colors.textPrimary} />
|
||||
<Text style={styles.markdownRefreshText}>Retry</Text>
|
||||
</Pressable>
|
||||
</View>
|
||||
)
|
||||
return <MobileLoadErrorState message={doc.message} onRetry={onRefresh} />
|
||||
}
|
||||
|
||||
const statusText = markdownReaderStatusText(doc)
|
||||
|
||||
@@ -100,26 +100,9 @@ export const mobileSessionReviewCommentStyles = StyleSheet.create({
|
||||
fontSize: typography.metaSize,
|
||||
fontWeight: '700'
|
||||
},
|
||||
markdownRefreshButton: {
|
||||
alignSelf: 'flex-start',
|
||||
flexDirection: 'row',
|
||||
alignItems: 'center',
|
||||
gap: spacing.xs,
|
||||
backgroundColor: colors.bgRaised,
|
||||
borderWidth: 1,
|
||||
borderColor: colors.borderSubtle,
|
||||
borderRadius: radii.button,
|
||||
paddingHorizontal: spacing.md,
|
||||
paddingVertical: spacing.xs
|
||||
},
|
||||
markdownButtonDisabled: {
|
||||
opacity: 0.45
|
||||
},
|
||||
markdownRefreshText: {
|
||||
color: colors.textPrimary,
|
||||
fontSize: 13,
|
||||
fontWeight: '600'
|
||||
},
|
||||
markdownFloatingBar: {
|
||||
position: 'absolute',
|
||||
left: spacing.md,
|
||||
|
||||
@@ -19,6 +19,7 @@ vi.mock('react-native', () => ({
|
||||
Text: 'Text',
|
||||
Pressable: 'Pressable',
|
||||
ActivityIndicator: 'ActivityIndicator',
|
||||
StyleSheet: { create: <T,>(styles: T) => styles, hairlineWidth: 1 },
|
||||
Platform: { OS: 'ios' },
|
||||
Keyboard: { dismiss: vi.fn() }
|
||||
}))
|
||||
|
||||
@@ -15,10 +15,6 @@ import {
|
||||
setupWindow
|
||||
} from './mobile-markdown-bridge-test-harness'
|
||||
|
||||
vi.mock('@/components/tab-bar/group-tab-order', () => ({
|
||||
getActiveTabNavOrder: () => [{ type: 'editor', id: '/repo/README.md', tabId: 'tab-md' }]
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/connection-context', () => ({
|
||||
getConnectionIdForFile: () => null
|
||||
}))
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { expect, vi } from 'vitest'
|
||||
import type { RuntimeMobileMarkdownRequest } from '../../../shared/mobile-markdown-document'
|
||||
import { useAppStore } from '../store'
|
||||
import type { Tab } from '../../../shared/tab-types'
|
||||
|
||||
type WindowStub = {
|
||||
addEventListener: Window['addEventListener']
|
||||
@@ -73,6 +74,25 @@ export function resetEditorState(): void {
|
||||
}
|
||||
]
|
||||
} as never)
|
||||
useAppStore.setState({
|
||||
unifiedTabsByWorktree: { 'wt-1': [markdownUnifiedTab('tab-md', '/repo/README.md', 'group-1')] }
|
||||
})
|
||||
}
|
||||
|
||||
/** The desktop tab strip entry a phone names a markdown tab by. */
|
||||
export function markdownUnifiedTab(id: string, fileId: string, groupId: string): Tab {
|
||||
return {
|
||||
id,
|
||||
entityId: fileId,
|
||||
groupId,
|
||||
worktreeId: 'wt-1',
|
||||
contentType: 'editor',
|
||||
label: fileId,
|
||||
customLabel: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 0
|
||||
}
|
||||
}
|
||||
|
||||
export function openMarkdownFile(): void {
|
||||
|
||||
@@ -11,16 +11,13 @@ import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../shared/constants'
|
||||
import { attachMobileMarkdownBridge } from './mobile-markdown-bridge'
|
||||
import {
|
||||
cleanupMobileMarkdownBridgeHarness,
|
||||
markdownUnifiedTab,
|
||||
openMarkdownFile,
|
||||
resetEditorState,
|
||||
sendRequest,
|
||||
setupWindow
|
||||
} from './mobile-markdown-bridge-test-harness'
|
||||
|
||||
vi.mock('@/components/tab-bar/group-tab-order', () => ({
|
||||
getActiveTabNavOrder: () => [{ type: 'editor', id: '/repo/README.md', tabId: 'tab-md' }]
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/connection-context', () => ({
|
||||
getConnectionIdForFile: () => null
|
||||
}))
|
||||
@@ -114,6 +111,72 @@ describe('mobile markdown bridge', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('reads a markdown tab that sits in a split group the desktop is not focused on', async () => {
|
||||
openMarkdownFile()
|
||||
useAppStore.setState({
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-1': [markdownUnifiedTab('tab-md', '/repo/README.md', 'group-split')]
|
||||
},
|
||||
groupsByWorktree: {
|
||||
'wt-1': [
|
||||
{ id: 'group-main', worktreeId: 'wt-1', activeTabId: null, tabOrder: [] },
|
||||
{
|
||||
id: 'group-split',
|
||||
worktreeId: 'wt-1',
|
||||
activeTabId: 'tab-md',
|
||||
tabOrder: ['tab-md']
|
||||
}
|
||||
]
|
||||
},
|
||||
activeGroupIdByWorktree: { 'wt-1': 'group-main' }
|
||||
})
|
||||
setupWindow({ readFile: vi.fn().mockResolvedValue({ content: '# split', isBinary: false }) })
|
||||
const detach = attachMobileMarkdownBridge()
|
||||
|
||||
try {
|
||||
const response = await sendRequest({
|
||||
id: 'read-split',
|
||||
operation: 'read',
|
||||
worktreeId: 'wt-1',
|
||||
tabId: 'tab-md'
|
||||
})
|
||||
|
||||
expect(response).toMatchObject({ ok: true, result: { content: '# split' } })
|
||||
} finally {
|
||||
detach()
|
||||
}
|
||||
})
|
||||
|
||||
it('reads the requested tab when the same file is open in two groups', async () => {
|
||||
openMarkdownFile()
|
||||
useAppStore.setState({
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-1': [
|
||||
markdownUnifiedTab('tab-main', '/repo/README.md', 'group-main'),
|
||||
markdownUnifiedTab('tab-split', '/repo/README.md', 'group-split')
|
||||
]
|
||||
}
|
||||
})
|
||||
setupWindow({ readFile: vi.fn().mockResolvedValue({ content: '# both', isBinary: false }) })
|
||||
const detach = attachMobileMarkdownBridge()
|
||||
|
||||
try {
|
||||
const response = await sendRequest({
|
||||
id: 'read-duplicate',
|
||||
operation: 'read',
|
||||
worktreeId: 'wt-1',
|
||||
tabId: 'tab-split'
|
||||
})
|
||||
|
||||
expect(response).toMatchObject({
|
||||
ok: true,
|
||||
result: { tabId: 'tab-split', filePath: '/repo/README.md', content: '# both' }
|
||||
})
|
||||
} finally {
|
||||
detach()
|
||||
}
|
||||
})
|
||||
|
||||
it('rejects save when a clean file changed after mobile read', async () => {
|
||||
openMarkdownFile()
|
||||
const writeFile = vi.fn().mockResolvedValue(undefined)
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import { getActiveTabNavOrder } from '@/components/tab-bar/group-tab-order'
|
||||
import {
|
||||
ORCA_EDITOR_FILE_SAVED_EVENT,
|
||||
requestEditorFileSave,
|
||||
@@ -188,13 +187,13 @@ function resolveMarkdownTarget(
|
||||
tabId: string
|
||||
): { tab: OpenFile; sourceFile: OpenFile } {
|
||||
const state = useAppStore.getState()
|
||||
const orderItem = getActiveTabNavOrder(state, worktreeId).find(
|
||||
(item) => item.type === 'editor' && (item.tabId === tabId || item.id === tabId)
|
||||
)
|
||||
const tabFileId = orderItem?.type === 'editor' ? orderItem.id : tabId
|
||||
const tab = state.openFiles.find(
|
||||
(file) => file.worktreeId === worktreeId && (file.id === tabFileId || file.id === tabId)
|
||||
// Why every group: the phone lists tabs from all split groups, not just the desktop's focused one.
|
||||
const unifiedTab = (state.unifiedTabsByWorktree[worktreeId] ?? []).find(
|
||||
(candidate) => candidate.id === tabId && candidate.contentType === 'editor'
|
||||
)
|
||||
// Hosts publish the file id when a file has no unified tab.
|
||||
const fileId = unifiedTab?.entityId ?? tabId
|
||||
const tab = state.openFiles.find((file) => file.worktreeId === worktreeId && file.id === fileId)
|
||||
if (!tab || !isMarkdownTab(tab)) {
|
||||
throw new Error('tab_not_found')
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user