From 43718e82074c53dbbc85ce557be8908fe84b0ac9 Mon Sep 17 00:00:00 2001 From: zhirongwang Date: Sat, 12 Sep 2026 23:54:25 -0700 Subject: [PATCH] feat(content-preview): forward comparison flags so Preview can own the banners Stamp isComparing and isComparedPreview on show() and on an already-open current pane so BCP can render comparison chrome without a reload. Co-authored-by: Cursor --- .../content-preview/ContentPreview.js | 22 ++++++ .../__tests__/ContentPreview.test.js | 78 +++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/src/elements/content-preview/ContentPreview.js b/src/elements/content-preview/ContentPreview.js index b2b371fa87..851887729e 100644 --- a/src/elements/content-preview/ContentPreview.js +++ b/src/elements/content-preview/ContentPreview.js @@ -138,6 +138,8 @@ type Props = { hasProviders?: boolean, hideSidebar?: boolean, isComparing?: boolean, + // Compared pane only. Do not also set isComparing — that flag drives the host layout slot. + isComparedPreview?: boolean, comparedSlotRef?: (?HTMLDivElement) => mixed, comparedVersion?: BoxItemVersion, isLarge: boolean, @@ -633,6 +635,22 @@ class ContentPreview extends React.PureComponent { this.setState({ selectedVersion: undefined }); } + // Comparison can start on an already-open current pane; stamp BCP without reloading. + const comparisonFlagsChanged = + !!prevProps.isComparing !== !!this.props.isComparing || + !!prevProps.isComparedPreview !== !!this.props.isComparedPreview; + if ( + comparisonFlagsChanged && + this.preview && + this.preview.setComparisonMode && + !this.shouldLoadPreview(prevProps, prevState) + ) { + this.preview.setComparisonMode({ + isComparing: !!(this.props.isComparing || this.props.isComparedPreview), + isComparedPreview: !!this.props.isComparedPreview, + }); + } + if (haveExperiencesChanged && this.preview && this.preview.updateExperiences) { this.preview.updateExperiences(previewExperiences); } @@ -1053,6 +1071,7 @@ class ContentPreview extends React.PureComponent { fileOptions, comparedSlotRef, isComparing, + isComparedPreview, onAnnotatorEvent, onAnnotator, onContentInsightsEventReport, @@ -1113,6 +1132,8 @@ class ContentPreview extends React.PureComponent { fileOptions: fileOpts, header: 'none', headerElement: `#${this.id} .bcpr-PreviewHeader`, + isComparing: !!(isComparing || isComparedPreview), + isComparedPreview: !!isComparedPreview, experiences: previewExperiences, preloadStatus, previewMode, @@ -1968,6 +1989,7 @@ function ContentPreviewWithComparison(props: ContentPreviewProps) { hasHeader={false} hideSidebar isComparing={false} + isComparedPreview // Hosts defer the indicator to let a preloaded image show through instead. // Nothing preloads the compared version, so deferring leaves this pane blank. loadingIndicatorDelayMs={0} diff --git a/src/elements/content-preview/__tests__/ContentPreview.test.js b/src/elements/content-preview/__tests__/ContentPreview.test.js index 07903b72f8..7591524c2e 100644 --- a/src/elements/content-preview/__tests__/ContentPreview.test.js +++ b/src/elements/content-preview/__tests__/ContentPreview.test.js @@ -462,6 +462,36 @@ describe('elements/content-preview/ContentPreview', () => { ); }); + test('should tell BCP the current pane is in a comparison session', async () => { + const wrapper = getWrapper({ ...props, isComparing: true }); + wrapper.setState({ file }); + const instance = wrapper.instance(); + await instance.loadPreview(); + expect(instance.preview.show).toHaveBeenCalledWith( + file.id, + expect.any(Function), + expect.objectContaining({ + isComparing: true, + isComparedPreview: false, + }), + ); + }); + + test('should tell BCP the compared pane is the older version', async () => { + const wrapper = getWrapper({ ...props, isComparedPreview: true }); + wrapper.setState({ file }); + const instance = wrapper.instance(); + await instance.loadPreview(); + expect(instance.preview.show).toHaveBeenCalledWith( + file.id, + expect.any(Function), + expect.objectContaining({ + isComparing: true, + isComparedPreview: true, + }), + ); + }); + test('should omit annotatorToken when token is a string', async () => { const wrapper = getWrapper(props); wrapper.setState({ file }); @@ -1528,6 +1558,36 @@ describe('elements/content-preview/ContentPreview', () => { expect(wrapper.state('selectedVersion')).toBeUndefined(); }); + + test('should stamp comparison flags on the live preview when comparison starts without a reload', () => { + instance.shouldLoadPreview = jest.fn().mockReturnValue(false); + instance.preview = { + setComparisonMode: jest.fn(), + updateExperiences: jest.fn(), + }; + + wrapper.setProps({ isComparing: true }); + + expect(instance.preview.setComparisonMode).toHaveBeenCalledWith({ + isComparing: true, + isComparedPreview: false, + }); + }); + + test('should not stamp comparison flags when the preview is reloading anyway', () => { + instance.shouldLoadPreview = jest.fn().mockReturnValue(true); + const livePreview = { + destroy: jest.fn(), + removeAllListeners: jest.fn(), + setComparisonMode: jest.fn(), + updateExperiences: jest.fn(), + }; + instance.preview = livePreview; + + wrapper.setProps({ isComparing: true }); + + expect(livePreview.setComparisonMode).not.toHaveBeenCalled(); + }); }); describe('getDerivedStateFromProps()', () => { @@ -2906,6 +2966,24 @@ describe('elements/content-preview/ContentPreview', () => { expect(wrapper.childAt(1).props().children.props.onMetric).not.toBe(onMetric); }); + test('should stamp isComparedPreview only on the compared instance', () => { + const wrapper = shallow( + , + ); + + wrapper.childAt(0).props().comparedSlotRef(document.createElement('div')); + wrapper.update(); + + expect(wrapper.childAt(0).props().isComparing).toBe(true); + expect(wrapper.childAt(0).props().isComparedPreview).toBeUndefined(); + expect(wrapper.childAt(1).props().children.props.isComparing).toBe(false); + expect(wrapper.childAt(1).props().children.props.isComparedPreview).toBe(true); + }); + test('should not navigate when isComparing', () => { const wrapper = getWrapper({ fileId: '456',