Skip to content

Commit

Permalink
Log point panel saves pending edits when removing condition (#10579)
Browse files Browse the repository at this point in the history
  • Loading branch information
bvaughn authored Jun 21, 2024
1 parent ca83c36 commit c88885f
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 8 deletions.
59 changes: 59 additions & 0 deletions packages/e2e-tests/tests/logpoints-12.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { openDevToolsTab, startTest } from "../helpers";
import { verifyConsoleMessage } from "../helpers/console-panel";
import { addLogpoint, editLogPoint, removeConditional } from "../helpers/source-panel";
import test from "../testFixture";

const lineNumber = 20;
test.use({ exampleKey: "doc_rr_basic.html" });

test(`logpoints-12: should auto save when removing conditions`, async ({
pageWithMeta: { page, recordingId, testScope },
exampleKey,
}) => {
await startTest(page, recordingId, testScope);
await openDevToolsTab(page);

await addLogpoint(page, {
content: '"initial"',
lineNumber,
saveAfterEdit: true,
url: exampleKey,
});
await verifyConsoleMessage(page, "initial", "log-point", 10);

// Add a condition that will hide console logs
await editLogPoint(page, {
condition: "false",
lineNumber,
saveAfterEdit: true,
url: exampleKey,
});
await verifyConsoleMessage(page, "initial", "log-point", 0);

// Remove condition and verify there are now console logs
await removeConditional(page, { lineNumber });
await verifyConsoleMessage(page, "initial", "log-point", 10);

// Re-add a condition that will hide console logs
await editLogPoint(page, {
condition: "false",
lineNumber,
saveAfterEdit: true,
url: exampleKey,
});

// Start a pending edit
await editLogPoint(page, {
content: '"updated"',
lineNumber,
saveAfterEdit: false,
url: exampleKey,
});

// Verify no console logs
await verifyConsoleMessage(page, "updated", "log-point", 0);

// Remove condition and verify the pending edit was also saved
await removeConditional(page, { lineNumber });
await verifyConsoleMessage(page, "updated", "log-point", 10);
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { TimeStampedPoint, TimeStampedPointRange } from "@replayio/protocol";
import {
MouseEvent,
Suspense,
unstable_useCacheRefresh as useCacheRefresh,
useContext,
Expand Down Expand Up @@ -245,11 +244,13 @@ export function PointPanelWithHitPoints({
}

if (hasCondition) {
editPendingPointText(key, { condition: null });

// TODO Clean this interaction up while we're in here
// If there's a pending text edit to content, we can save only the condition change
// If we're removing a condition, we need to account for pending partial edits
// Ideally we would stash them, save the log point without a condition, and then reapply them
// But the save+render cycle is async so it's easiest to just save the pending edit along with the condition change
savePendingPointText(key, { condition: null, content });
setIsEditing(false);
} else {
// If we're adding a condition, just focus to the condition field
if (!isEditing) {
startEditing("condition");
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ export default function useSavePendingPointText({
setPointBehaviors: SetLocalPointBehaviors;
}) {
return useCallback<SaveOrDiscardPendingText>(
(key: PointKey) => {
(key: PointKey, partialPoint?: Partial<Pick<Point, "condition" | "content">>) => {
const { pendingPointText } = committedValuesRef.current;
const pendingPoint = pendingPointText.get(key);
const pendingPoint = partialPoint ?? pendingPointText.get(key);
if (pendingPoint) {
saveLocalAndRemotePoints(key, pendingPoint);

Expand Down
5 changes: 4 additions & 1 deletion packages/replay-next/src/contexts/points/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ export type EditPointBehavior = (
createdByCurrentUser: boolean
) => void;

export type SaveOrDiscardPendingText = (key: PointKey) => void;
export type SaveOrDiscardPendingText = (
key: PointKey,
partialPoint?: Partial<Pick<Point, "condition" | "content">>
) => void;

export type SaveLocalAndRemotePoints = (
key: PointKey,
Expand Down

0 comments on commit c88885f

Please sign in to comment.