fix(mobile): isolate consecutive shares into separate notes
This commit is contained in:
1 parent
9136841538
commit
680fae0e5a
6 files changed
+258
-17
No files matched your search
@@ -29,6 +29,7 @@ struct MemoEditView: View {
|
||||
let mode: MemoEditMode
|
||||
var initialFocus: MemoEditInitialFocus = .body
|
||||
var initialSharedImages: [ShareHandoffStore.SharedImage] = []
|
||||
var finishForIncomingShare = false
|
||||
/// When set (edit-from-detail), close by popping to the list under the cover first —
|
||||
/// never `dismiss()` onto a still-pushed detail page.
|
||||
var onLeaveToList: (() -> Void)? = nil
|
||||
@@ -58,6 +59,8 @@ struct MemoEditView: View {
|
||||
@State private var showEmptyAiSelectionAlert = false
|
||||
@State private var aiUndoToken: UUID?
|
||||
@State private var didImportSharedImages = false
|
||||
@State private var initialShareImportFinished = false
|
||||
@State private var shareHandoffStarted = false
|
||||
@State private var isSuggestingTags = false
|
||||
@State private var smartTagsAdded = false
|
||||
@State private var showSmartTagAlert = false
|
||||
@@ -312,6 +315,14 @@ struct MemoEditView: View {
|
||||
}
|
||||
}
|
||||
await importInitialSharedImagesIfNeeded()
|
||||
initialShareImportFinished = true
|
||||
}
|
||||
.onChange(of: canFinishForIncomingShare, initial: true) { _, ready in
|
||||
guard ready else { return }
|
||||
// Use the normal flush/commit path. Failure keeps the current cover
|
||||
// and queued shares intact; manual Done/Back can retry the save.
|
||||
shareHandoffStarted = true
|
||||
Task { await handleDone() }
|
||||
}
|
||||
.onDisappear {
|
||||
smartTagTask?.cancel()
|
||||
@@ -330,6 +341,13 @@ struct MemoEditView: View {
|
||||
|
||||
// MARK: - Header (createMemoHeader)
|
||||
|
||||
private var canFinishForIncomingShare: Bool {
|
||||
isCreate && finishForIncomingShare && !shareHandoffStarted
|
||||
&& contentHydrated && editorReady && initialShareImportFinished
|
||||
&& !isUploading && !isMaterializing && !isCreating && !isSaving
|
||||
&& error == nil && !showUploadError
|
||||
}
|
||||
|
||||
private var createHeader: some View {
|
||||
HStack(spacing: 8) {
|
||||
Button {
|
||||
|
||||
@@ -17,14 +17,47 @@ private struct SelectionSharePayload: Identifiable {
|
||||
let url: URL
|
||||
}
|
||||
|
||||
/// Keep each create editor's input fixed until its save and cover dismissal finish.
|
||||
struct MemoCreatePresentation {
|
||||
struct Request: Identifiable {
|
||||
let id = UUID()
|
||||
let seed: CreateMemoSeed?
|
||||
let sharedImages: [ShareHandoffStore.SharedImage]
|
||||
|
||||
init(seed: CreateMemoSeed?, sharedImages: [ShareHandoffStore.SharedImage] = []) {
|
||||
self.seed = seed
|
||||
self.sharedImages = sharedImages
|
||||
}
|
||||
}
|
||||
|
||||
private(set) var current: Request?
|
||||
private(set) var pending: [Request] = []
|
||||
var shouldFinishForIncomingShare: Bool { !pending.isEmpty }
|
||||
|
||||
/// Returns true only when a new cover can be presented immediately.
|
||||
mutating func open(_ request: Request) -> Bool {
|
||||
guard current == nil else {
|
||||
pending.append(request)
|
||||
return false
|
||||
}
|
||||
current = request
|
||||
return true
|
||||
}
|
||||
|
||||
/// Called by fullScreenCover's onDismiss, after the old editor has gone away.
|
||||
mutating func didDismiss() -> Bool {
|
||||
current = pending.isEmpty ? nil : pending.removeFirst()
|
||||
return current != nil
|
||||
}
|
||||
}
|
||||
|
||||
struct WorkspaceView: View {
|
||||
@Environment(AppEnvironment.self) private var env
|
||||
@State private var store = WorkspaceStore()
|
||||
@State private var path = NavigationPath()
|
||||
@State private var showSettings = false
|
||||
@State private var showNewNote = false
|
||||
@State private var createSeed: CreateMemoSeed?
|
||||
@State private var createSharedImages: [ShareHandoffStore.SharedImage] = []
|
||||
@State private var createPresentation = MemoCreatePresentation()
|
||||
@State private var showCreateChoice = false
|
||||
@State private var showTemplatePicker = false
|
||||
@State private var createLongPressConsumed = false
|
||||
@@ -92,23 +125,22 @@ struct WorkspaceView: View {
|
||||
memoDetail(for: route.memoId, shareImage: true)
|
||||
}
|
||||
// Android CreateMemoModal is fullScreen — not a half sheet / Form.
|
||||
.fullScreenCover(isPresented: $showNewNote) {
|
||||
.fullScreenCover(isPresented: $showNewNote, onDismiss: {
|
||||
showNewNote = createPresentation.didDismiss()
|
||||
store.reload(env: env)
|
||||
}) {
|
||||
MemoEditView(
|
||||
mode: .create(
|
||||
notebookId: store.selectedNotebookId ?? store.notebooks.first?.id ?? "",
|
||||
seed: createSeed
|
||||
seed: createPresentation.current?.seed
|
||||
),
|
||||
initialSharedImages: createSharedImages,
|
||||
initialSharedImages: createPresentation.current?.sharedImages ?? [],
|
||||
finishForIncomingShare: createPresentation.shouldFinishForIncomingShare,
|
||||
onCreateFinished: { _ in
|
||||
store.reload(env: env)
|
||||
}
|
||||
)
|
||||
.onDisappear {
|
||||
createSeed = nil
|
||||
createSharedImages = []
|
||||
// Safety refresh if create finished without callback (e.g. swipe-dismiss empty).
|
||||
store.reload(env: env)
|
||||
}
|
||||
.id(createPresentation.current?.id)
|
||||
}
|
||||
.sheet(isPresented: $showCreateChoice) {
|
||||
CreateChoiceSheet(
|
||||
@@ -776,9 +808,9 @@ struct WorkspaceView: View {
|
||||
sharedImages: [ShareHandoffStore.SharedImage] = []
|
||||
) {
|
||||
guard !store.notebooks.isEmpty else { return }
|
||||
createSeed = seed
|
||||
createSharedImages = sharedImages
|
||||
showNewNote = true
|
||||
if createPresentation.open(.init(seed: seed, sharedImages: sharedImages)) {
|
||||
showNewNote = true
|
||||
}
|
||||
}
|
||||
|
||||
private func openCreateFromTemplate() {
|
||||
|
||||
@@ -4,6 +4,59 @@ import XCTest
|
||||
/// Drives the shipped create-Done path (`MemoCreateCommit.commit`).
|
||||
/// Guards Android parity: after image materialize, Done must update — not second create.
|
||||
final class MemoCreateCommitTests: XCTestCase {
|
||||
func testConsecutiveSharesKeepCurrentInputUntilCoverDismisses() throws {
|
||||
var presentation = MemoCreatePresentation()
|
||||
let article = MemoCreatePresentation.Request(
|
||||
seed: CreateMemoSeed(title: "Zhihu thought", contentMarkdown: "First share", tagsText: "")
|
||||
)
|
||||
let image = ShareHandoffStore.SharedImage(
|
||||
fileURL: URL(fileURLWithPath: "/tmp/second-share.jpg"),
|
||||
filename: "second-share.jpg",
|
||||
mimeType: "image/jpeg"
|
||||
)
|
||||
let next = MemoCreatePresentation.Request(
|
||||
seed: CreateMemoSeed(title: "Shared image", contentMarkdown: "", tagsText: ""),
|
||||
sharedImages: [image]
|
||||
)
|
||||
XCTAssertTrue(presentation.open(article))
|
||||
XCTAssertFalse(presentation.shouldFinishForIncomingShare)
|
||||
XCTAssertFalse(presentation.open(next))
|
||||
// A pending share or failed save must not change the mounted editor's inputs.
|
||||
XCTAssertEqual(presentation.current?.id, article.id)
|
||||
XCTAssertEqual(presentation.current?.seed, article.seed)
|
||||
XCTAssertEqual(presentation.current?.sharedImages, [])
|
||||
XCTAssertTrue(presentation.shouldFinishForIncomingShare)
|
||||
XCTAssertEqual(presentation.pending.count, 1)
|
||||
|
||||
XCTAssertTrue(presentation.didDismiss())
|
||||
XCTAssertEqual(presentation.current?.id, next.id)
|
||||
XCTAssertNotEqual(presentation.current?.id, article.id)
|
||||
XCTAssertEqual(presentation.current?.sharedImages, [image])
|
||||
XCTAssertEqual(presentation.current?.seed?.contentMarkdown, "")
|
||||
XCTAssertFalse(presentation.shouldFinishForIncomingShare)
|
||||
XCTAssertFalse(presentation.didDismiss())
|
||||
XCTAssertNil(presentation.current)
|
||||
}
|
||||
|
||||
func testAdditionalSharesArePresentedInOrderAfterEachDismissal() {
|
||||
var presentation = MemoCreatePresentation()
|
||||
let requests = (1...3).map { index in
|
||||
MemoCreatePresentation.Request(
|
||||
seed: CreateMemoSeed(title: "Share \(index)", contentMarkdown: "", tagsText: "")
|
||||
)
|
||||
}
|
||||
XCTAssertTrue(presentation.open(requests[0]))
|
||||
XCTAssertFalse(presentation.open(requests[1]))
|
||||
XCTAssertFalse(presentation.open(requests[2]))
|
||||
XCTAssertTrue(presentation.didDismiss())
|
||||
XCTAssertEqual(presentation.current?.id, requests[1].id)
|
||||
XCTAssertTrue(presentation.shouldFinishForIncomingShare)
|
||||
XCTAssertTrue(presentation.didDismiss())
|
||||
XCTAssertEqual(presentation.current?.id, requests[2].id)
|
||||
XCTAssertFalse(presentation.shouldFinishForIncomingShare)
|
||||
XCTAssertFalse(presentation.didDismiss())
|
||||
}
|
||||
|
||||
private var mirror: LocalMirrorRepository!
|
||||
private var outbox: SyncOutboxRepository!
|
||||
private var drafts: DraftRepository!
|
||||
|
||||
@@ -168,7 +168,7 @@ describe("incoming share hook", () => {
|
||||
test("workspace finishes the accepted clip when resolved payloads rerun its effect", async () => {
|
||||
const source = readFileSync(new URL("../screens/WorkspaceScreen.tsx", import.meta.url), "utf8");
|
||||
const start = source.indexOf(" useEffect(() => {\n if (incomingShareIsResolving)");
|
||||
const end = source.indexOf(" }, [\n incomingShareError,", start);
|
||||
const end = source.indexOf(" }, [\n createOpen,", start);
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
const effectBody = source.slice(start + " useEffect(() => {".length, end);
|
||||
@@ -177,6 +177,7 @@ test("workspace finishes the accepted clip when resolved payloads rerun its effe
|
||||
let importing = false;
|
||||
let handled = 0;
|
||||
const context = {
|
||||
createOpen: false,
|
||||
incomingShareIsResolving: false,
|
||||
incomingShareError: null,
|
||||
incomingSharePayloads: article,
|
||||
@@ -209,3 +210,98 @@ test("workspace finishes the accepted clip when resolved payloads rerun its effe
|
||||
expect(importing).toBe(false);
|
||||
expect(handled).toBe(1);
|
||||
});
|
||||
|
||||
describe("consecutive shares while the create editor stays open", () => {
|
||||
const source = readFileSync(new URL("../screens/WorkspaceScreen.tsx", import.meta.url), "utf8");
|
||||
const start = source.indexOf(" useEffect(() => {\n if (incomingShareIsResolving)");
|
||||
const end = source.indexOf(" }, [\n createOpen,", start);
|
||||
const effectBody = source.slice(start + " useEffect(() => {".length, end);
|
||||
|
||||
test.each(["article", "image"])("holds a new %s share until the previous note is saved and unmounted", (kind) => {
|
||||
const images = kind === "image" ? [{ uri: "file:///cache/second.jpg" }] : [];
|
||||
let finishRequested = false;
|
||||
let nextEditorOpened = false;
|
||||
let handled = 0;
|
||||
const previousImages: unknown[] = [];
|
||||
let nextImages: unknown[] = previousImages;
|
||||
const context = {
|
||||
createOpen: true,
|
||||
incomingShareIsResolving: false,
|
||||
incomingShareError: null,
|
||||
incomingSharePayloads: article,
|
||||
incomingShareUrl: article[0]!.value,
|
||||
sharedImages: images,
|
||||
notebooks: [{}],
|
||||
notebooksQuery: { isSuccess: true },
|
||||
processedShareUrlRef: { current: "https://www.zhihu.com/pin/previous" },
|
||||
setFinishCreateForIncomingShare: (value: boolean) => { finishRequested = value; },
|
||||
beginEditorStartup: () => {},
|
||||
setSelectedMemoId: () => {},
|
||||
setIncomingClipDraft: () => {},
|
||||
setIncomingShareImages: (value: unknown[]) => { nextImages = value; },
|
||||
setCreateSeed: () => {},
|
||||
setActiveView: () => {},
|
||||
setMemoView: () => {},
|
||||
setCreateOpen: (value: boolean) => { nextEditorOpened = value; },
|
||||
translate: (value: string) => value,
|
||||
onIncomingShareHandledRef: { current: () => { handled++; } },
|
||||
};
|
||||
const run = () => new Function(...Object.keys(context), effectBody)(...Object.values(context));
|
||||
run();
|
||||
expect(finishRequested).toBe(true);
|
||||
expect(nextEditorOpened).toBe(false);
|
||||
expect(nextImages).toBe(previousImages);
|
||||
expect(handled).toBe(0);
|
||||
expect(context.processedShareUrlRef.current).toBe("https://www.zhihu.com/pin/previous");
|
||||
if (kind === "image") {
|
||||
context.createOpen = false; // Only a successful save closes the old editor.
|
||||
run();
|
||||
expect(nextEditorOpened).toBe(true);
|
||||
expect(nextImages).toEqual(images);
|
||||
expect(handled).toBe(1);
|
||||
}
|
||||
});
|
||||
|
||||
test("waits for editor readiness and the entire image batch, then saves once without retrying failures", () => {
|
||||
const editor = readFileSync(new URL("../screens/WorkspaceEditors.tsx", import.meta.url), "utf8");
|
||||
const start = editor.indexOf(" useEffect(() => {\n if (!finishForIncomingShare)");
|
||||
const end = editor.indexOf(" }, [finishForIncomingShare,", start);
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
const body = editor.slice(start + " useEffect(() => {".length, end);
|
||||
let saves = 0;
|
||||
const context = {
|
||||
finishForIncomingShare: true,
|
||||
draftLoaded: false,
|
||||
editorReady: false,
|
||||
importingSharedImages: true,
|
||||
sharedImageImportFailed: false,
|
||||
imageOperation: "idle",
|
||||
submitStarted: false,
|
||||
shareHandoffStartedRef: { current: false },
|
||||
submitCreateMemo: () => { saves++; },
|
||||
};
|
||||
const run = () => new Function(...Object.keys(context), body)(...Object.values(context));
|
||||
run();
|
||||
context.draftLoaded = true;
|
||||
context.editorReady = true;
|
||||
run(); // idle between two images must not close the editor
|
||||
expect(saves).toBe(0);
|
||||
context.importingSharedImages = false;
|
||||
context.imageOperation = "uploading";
|
||||
run();
|
||||
expect(saves).toBe(0);
|
||||
context.imageOperation = "idle";
|
||||
context.sharedImageImportFailed = true;
|
||||
run(); // an upload error must leave the previous note open
|
||||
expect(saves).toBe(0);
|
||||
context.sharedImageImportFailed = false;
|
||||
run();
|
||||
expect(saves).toBe(1);
|
||||
context.submitStarted = true;
|
||||
run();
|
||||
context.submitStarted = false; // save failed: preserve the old editor for retry
|
||||
run();
|
||||
expect(saves).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -144,6 +144,7 @@ export const CreateMemoModal = ({
|
||||
client: clientProp,
|
||||
dataScope,
|
||||
defaultNotebookId,
|
||||
finishForIncomingShare = false,
|
||||
imageCompressionEnabled,
|
||||
initialDraft,
|
||||
initialSharedImages = [],
|
||||
@@ -156,6 +157,7 @@ export const CreateMemoModal = ({
|
||||
client?: ReturnType<typeof useSession>["client"];
|
||||
dataScope: string;
|
||||
defaultNotebookId: string;
|
||||
finishForIncomingShare?: boolean;
|
||||
imageCompressionEnabled: boolean;
|
||||
initialDraft?: MobileCreateMemoSeed | MobileWebClipDraft | null;
|
||||
initialSharedImages?: MobileSharedImage[];
|
||||
@@ -200,6 +202,9 @@ export const CreateMemoModal = ({
|
||||
const [imageOperation, setImageOperation] = useState<"idle" | "creating" | "uploading">("idle");
|
||||
const imageOperationRef = useRef(imageOperation);
|
||||
const sharedImagesHandledRef = useRef(false);
|
||||
const [importingSharedImages, setImportingSharedImages] = useState(initialSharedImages.length > 0);
|
||||
const [sharedImageImportFailed, setSharedImageImportFailed] = useState(false);
|
||||
const shareHandoffStartedRef = useRef(false);
|
||||
const createPendingRef = useRef(false);
|
||||
const submitStartedRef = useRef(false);
|
||||
const [submitStarted, setSubmitStarted] = useState(false);
|
||||
@@ -566,8 +571,14 @@ export const CreateMemoModal = ({
|
||||
}
|
||||
sharedImagesHandledRef.current = true;
|
||||
void (async () => {
|
||||
for (const image of initialSharedImages) {
|
||||
await uploadImageAsset(image);
|
||||
try {
|
||||
for (const image of initialSharedImages) {
|
||||
if (await uploadImageAsset(image) === undefined) {
|
||||
setSharedImageImportFailed(true);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
setImportingSharedImages(false);
|
||||
}
|
||||
})();
|
||||
}, [editorReady, initialSharedImages]);
|
||||
@@ -603,6 +614,21 @@ export const CreateMemoModal = ({
|
||||
|
||||
const requestClose = () => submitCreateMemo();
|
||||
|
||||
useEffect(() => {
|
||||
if (!finishForIncomingShare) {
|
||||
shareHandoffStartedRef.current = false;
|
||||
return;
|
||||
}
|
||||
if (!draftLoaded || !editorReady || importingSharedImages || sharedImageImportFailed || imageOperation !== "idle" || submitStarted || shareHandoffStartedRef.current) {
|
||||
return;
|
||||
}
|
||||
// Save through the normal close path, including editor flush and queued
|
||||
// writes. On failure keep both this editor and the next share intact;
|
||||
// the user can retry saving without an automatic retry loop.
|
||||
shareHandoffStartedRef.current = true;
|
||||
void submitCreateMemo();
|
||||
}, [finishForIncomingShare, draftLoaded, editorReady, importingSharedImages, sharedImageImportFailed, imageOperation, submitStarted, submitCreateMemo]);
|
||||
|
||||
useEffect(() => {
|
||||
const subscription = BackHandler.addEventListener("hardwareBackPress", () => {
|
||||
void requestClose();
|
||||
|
||||
@@ -159,6 +159,7 @@ export const WorkspaceScreen = ({
|
||||
const [imageShareFromList, setImageShareFromList] = useState(false);
|
||||
const [searchText, setSearchText] = useState("");
|
||||
const [createOpen, setCreateOpen] = useState(false);
|
||||
const [finishCreateForIncomingShare, setFinishCreateForIncomingShare] = useState(false);
|
||||
const [createSeed, setCreateSeed] = useState<MobileCreateMemoSeed | null>(null);
|
||||
const [createChoiceOpen, setCreateChoiceOpen] = useState(false);
|
||||
const [templatePickerOpen, setTemplatePickerOpen] = useState(false);
|
||||
@@ -602,6 +603,17 @@ export const WorkspaceScreen = ({
|
||||
return;
|
||||
}
|
||||
|
||||
// A second external share must not replace props on the mounted create
|
||||
// editor: its document and upload target belong to the previous note.
|
||||
// Keep the new payload pending until that editor has saved and unmounted.
|
||||
const pendingShareKey = sharedImages.length > 0
|
||||
? `images:${sharedImages.map((image) => image.uri).join("|")}`
|
||||
: incomingShareUrl;
|
||||
if (createOpen && pendingShareKey && processedShareUrlRef.current !== pendingShareKey && notebooks.length > 0) {
|
||||
setFinishCreateForIncomingShare(true);
|
||||
return;
|
||||
}
|
||||
|
||||
if (sharedImages.length > 0) {
|
||||
const shareKey = `images:${sharedImages.map((image) => image.uri).join("|")}`;
|
||||
if (processedShareUrlRef.current === shareKey) {
|
||||
@@ -690,6 +702,7 @@ export const WorkspaceScreen = ({
|
||||
}
|
||||
});
|
||||
}, [
|
||||
createOpen,
|
||||
incomingShareError,
|
||||
incomingShareIsResolving,
|
||||
incomingSharePayloads.length,
|
||||
@@ -1203,11 +1216,13 @@ export const WorkspaceScreen = ({
|
||||
client={client}
|
||||
dataScope={dataScope}
|
||||
defaultNotebookId={createMemoNotebookId}
|
||||
finishForIncomingShare={finishCreateForIncomingShare}
|
||||
imageCompressionEnabled={imageCompressionEnabled}
|
||||
initialDraft={incomingClipDraft ?? createSeed}
|
||||
initialSharedImages={incomingShareImages}
|
||||
notebooks={notebooks}
|
||||
onCreated={() => {
|
||||
setFinishCreateForIncomingShare(false);
|
||||
setCreateOpen(false);
|
||||
setIncomingClipDraft(null);
|
||||
setIncomingShareImages([]);
|
||||
@@ -1217,6 +1232,7 @@ export const WorkspaceScreen = ({
|
||||
setSelectedMemoId(null);
|
||||
}}
|
||||
onDismiss={() => {
|
||||
setFinishCreateForIncomingShare(false);
|
||||
setCreateOpen(false);
|
||||
setIncomingClipDraft(null);
|
||||
setIncomingShareImages([]);
|
||||
|
||||
Reference in new issue
Block a user