Skip to content

Commit

Permalink
Change header for unchanged notebook meta
Browse files Browse the repository at this point in the history
We previously changed to always showing the metadata diff in notebook merges, in case the user needs to make manual edits to it even if there are no changes in the merge itself. That change did not take into account that the display behavior here made some assumptions about only being shown when there was a change. Here we make the title conditional on whether there are changes or not, and make sure not to collapse similar changes in the case where there are none.
  • Loading branch information
vidartf committed Nov 17, 2023
1 parent 2f856da commit 68e1c26
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
15 changes: 12 additions & 3 deletions packages/nbdime/src/merge/widget/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { MergePanel } from '../../common/basepanel';

import type { MetadataMergeModel } from '../model';

const ROOT_METADATA_CLASS = 'jp-Metadata-diff';
const ROOT_METADATA_CLASS = 'jp-Metadata-merge';

/**
* MetadataWidget for changes to Notebook-level metadata
Expand All @@ -36,18 +36,27 @@ export class MetadataMergeWidget extends MergePanel<MetadataMergeModel> {

// We know/assume that MetadataMergeModel never has
// null values for local/remote:
const viewOptions = {...this._viewOptions};
const unchanged = model.decisions.length === 0;
if (unchanged) {
viewOptions.collapseIdentical = false;
}
this.view = createNbdimeMergeView({
remote: model.remote,
local: model.local,
merged: model.merged,
factory: this._editorFactory,
translator: this._translator,
...this._viewOptions,
...viewOptions,
});
const trans = this._translator.load('nbdime');
const wrapper = new CollapsiblePanel(
this.view,
trans.__('Notebook metadata changed'),
trans.__(
`Notebook metadata ${
unchanged ? 'unchanged' : 'changed'
}`,
),
true,
);
this.addWidget(wrapper);
Expand Down
4 changes: 2 additions & 2 deletions packages/nbdime/src/styles/merge.css
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
.jp-Notebook-merge .jp-Metadata-diff {
.jp-Notebook-merge .jp-Metadata-merge {
margin-bottom: 20px;
}

.jp-Notebook-merge .jp-Metadata-diff .jp-CollapsiblePanel {
.jp-Notebook-merge .jp-Metadata-merge .jp-CollapsiblePanel {
border: solid black thin;
}

Expand Down
5 changes: 5 additions & 0 deletions ui-tests/tests/nbdime-merge-test1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ test.describe('merge test1', () => {
// Finalize download
expect(await download1.failure()).toBeNull();
});

test('should not collapse source for unchanged metadata', async ({ page }) => {
await page.locator('.jp-Metadata-merge .jp-CollapsiblePanel-header-icon').click();
expect(await page.locator('#main').screenshot()).toMatchSnapshot();

Check failure on line 76 in ui-tests/tests/nbdime-merge-test1.spec.ts

View workflow job for this annotation

GitHub Actions / ui-test (ubuntu-22.04)

tests/nbdime-merge-test1.spec.ts:74:7 › merge test1 › should not collapse source for unchanged metadata

5) tests/nbdime-merge-test1.spec.ts:74:7 › merge test1 › should not collapse source for unchanged metadata Error: A snapshot doesn't exist at /home/runner/work/nbdime/nbdime/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-should-not-collapse-source-for-unchanged-metadata-1-linux.png. 74 | test('should not collapse source for unchanged metadata', async ({ page }) => { 75 | await page.locator('.jp-Metadata-merge .jp-CollapsiblePanel-header-icon').click(); > 76 | expect(await page.locator('#main').screenshot()).toMatchSnapshot(); | ^ 77 | }); 78 | }); 79 | at /home/runner/work/nbdime/nbdime/ui-tests/tests/nbdime-merge-test1.spec.ts:76:54

Check failure on line 76 in ui-tests/tests/nbdime-merge-test1.spec.ts

View workflow job for this annotation

GitHub Actions / ui-test (ubuntu-22.04)

tests/nbdime-merge-test1.spec.ts:74:7 › merge test1 › should not collapse source for unchanged metadata

5) tests/nbdime-merge-test1.spec.ts:74:7 › merge test1 › should not collapse source for unchanged metadata Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: A snapshot doesn't exist at /home/runner/work/nbdime/nbdime/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-should-not-collapse-source-for-unchanged-metadata-1-linux.png, writing actual. 74 | test('should not collapse source for unchanged metadata', async ({ page }) => { 75 | await page.locator('.jp-Metadata-merge .jp-CollapsiblePanel-header-icon').click(); > 76 | expect(await page.locator('#main').screenshot()).toMatchSnapshot(); | ^ 77 | }); 78 | }); 79 | at /home/runner/work/nbdime/nbdime/ui-tests/tests/nbdime-merge-test1.spec.ts:76:54
});
});

test('3 panels view', async ({ page }) => {
Expand Down

0 comments on commit 68e1c26

Please sign in to comment.