Merge pull request #349 from visdesignlab/309-dedup-overlap-trrack

Ensure overlap degree pickers don't produce duplicate Trrack actions
visdesignlab · Apr 18, 2024 · bcbd08a · bcbd08a
2 parents 8bf9557 + e473fa1
commit bcbd08a
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 0 deletions.
diff --git a/e2e-tests/provenance.spec.ts b/e2e-tests/provenance.spec.ts
@@ -79,3 +79,52 @@ test('Selection History', async ({ page }) => {
   await expect(page.getByText('Bookmark Duff Fan & Male', { exact: true })).toBeVisible();
   await expect(page.getByText('Select intersection "Duff Fan')).toBeVisible();
 });
+
+/**
+ * Tests that overlap history works 
+ * & doesn't produce duplicate actions when the overlap degree is changed to the same value
+ */
+test('Overlap History', async ({ page }) => {
+  await page.goto('http://localhost:3000/?workspace=Upset+Examples&table=simpsons&sessionId=193');
+  await page.getByLabel('Open additional options menu').click();
+  await page.getByLabel('Open history tree sidebar').click();
+
+  // Ensure that duplicate actions aren't recorded for decreasing first degree
+  await page.getByRole('radio', { name: 'Overlaps' }).check();
+  await page.getByRole('spinbutton', { name: 'Degree' }).click();
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowDown');
+  await expect(page.getByText('First overlap by 2')).toHaveCount(0);
+
+  // Try to increment degree to 7 (limit is 6); confirm no duplicate at any level
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await expect(page.getByText('First overlap by 3')).toHaveCount(1);
+  await expect(page.getByText('First overlap by 4')).toHaveCount(1);
+  await expect(page.getByText('First overlap by 5')).toHaveCount(1);
+  await expect(page.getByText('First overlap by 6')).toHaveCount(1);
+
+  // Switch to second aggregation by overlap
+  await page.getByRole('radio', { name: 'Deviations' }).check();
+  await page.getByRole('button', { name: 'Second Aggregation' }).click();
+  await page.locator('div').filter({ hasText: /^Second AggregationDegreeSetsOverlapsNone$/ })
+    .getByLabel('Overlaps', { exact: true }).check();
+  await page.getByRole('spinbutton', { name: 'Degree' }).click();
+
+  // Ensure no duplicate action for decreasing 2nd degree
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowDown');
+  await expect(page.getByText('Second overlap by 2')).toHaveCount(0);
+
+  // Try to increment degree to 7; confirm no duplicates
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await page.getByRole('spinbutton', { name: 'Degree' }).press('ArrowUp');
+  await expect(page.getByText('Second overlap by 3')).toHaveCount(1);
+  await expect(page.getByText('Second overlap by 4')).toHaveCount(1);
+  await expect(page.getByText('Second overlap by 5')).toHaveCount(1);
+  await expect(page.getByText('Second overlap by 6')).toHaveCount(1);
+})
diff --git a/packages/upset/src/components/Sidebar.tsx b/packages/upset/src/components/Sidebar.tsx
@@ -131,8 +131,10 @@ export const Sidebar = () => {
                       value={firstOverlapDegree}
                       onChange={(ev) => {
                         let val = parseInt(ev.target.value, 10);
+                        if (!val) return; // Blocks users from clearing the input
                         if (val < 2) val = 2;
                         if (val > visibleSets.length) val = visibleSets.length;
+                        if (val === firstOverlapDegree) return; // Don't dispatch action if overlap hasn't changed
                         actions.firstOverlapBy(val);
                       }}
                     />
@@ -193,8 +195,10 @@ export const Sidebar = () => {
                         value={secondOverlapDegree}
                         onChange={(ev) => {
                           let val = parseInt(ev.target.value, 10);
+                          if (!val) return; // Block users from clearing the input
                           if (val < 2) val = 2;
                           if (val > visibleSets.length) val = visibleSets.length;
+                          if (val === secondOverlapDegree) return; // Don't dispatch an action if overlap hasn't changed
                           actions.secondOverlapBy(val);
                         }}
                       />