fix(web): keep the open MR footer panel through an in-place refresh
4 files · +33 -5
M internal/adapters/web/jstest/mr.test.mjs +22 -3
| @@ -97,7 +97,8 @@ function footerHarness(extra = {}) { | |||
| 97 | 97 | const saved = Object.fromEntries(['document', 'location', 'addEventListener', 'history', 'MutationObserver'].map((k) => [k, globalThis[k]])); | |
| 98 | 98 | const node = (id) => ({ id, attrs: {}, events: {}, hidden: false, | |
| 99 | 99 | setAttribute(k, v) { this.attrs[k] = v; }, getAttribute(k) { return this.attrs[k] ?? null; }, | |
| 100 | - | addEventListener(k, v) { this.events[k] = v; }, focus() { this.focused = true; } }); | |
| 100 | + | addEventListener(k, v) { this.events[k] = v; }, focus() { this.focused = true; }, | |
| 101 | + | scrollIntoView() { this.scrolled = (this.scrolled ?? 0) + 1; } }); | |
| 101 | 102 | const tabs = [node('mr-tab-commits'), node('mr-tab-checks')]; | |
| 102 | 103 | const panels = [node('mr-commits'), node('mr-checks-panel')]; | |
| 103 | 104 | const footer = node('footer'); footer.querySelectorAll = () => panels; | |
| @@ -110,7 +111,7 @@ function footerHarness(extra = {}) { | |||
| 110 | 111 | globalThis.history = { replaceState: (...args) => replaced.push(args) }; | |
| 111 | 112 | const restore = () => { for (const [k, v] of Object.entries(saved)) { if (v === undefined) delete globalThis[k]; else globalThis[k] = v; } }; | |
| 112 | 113 | const state = () => panels.map((p) => !p.hidden); | |
| 113 | - | return { tabs, panels, listeners, replaced, restore, state }; | |
| 114 | + | return { tabs, panels, footer, listeners, replaced, restore, state }; | |
| 114 | 115 | } | |
| 115 | 116 | ||
| 116 | 117 | test('footer tabs: closed by default, toggle, switch, arrows, hash', () => { | |
| @@ -178,15 +179,33 @@ test('footer: #mr-ck syncs the checks label; its click opens Checks without togg | |||
| 178 | 179 | test('footer: panels(index) restores the open panel without touching the URL', () => { | |
| 179 | 180 | const h = footerHarness(); | |
| 180 | 181 | try { | |
| 182 | + | location.hash = '#mr-commits'; | |
| 181 | 183 | window.ermiteMR.panels(1); | |
| 182 | - | assert.deepEqual(h.state(), [false, true]); | |
| 184 | + | assert.deepEqual(h.state(), [false, true], 'a restore ignores the fragment'); | |
| 183 | 185 | assert.equal(h.tabs[1].attrs['aria-expanded'], 'true'); | |
| 184 | 186 | assert.deepEqual(h.replaced, []); | |
| 187 | + | assert.equal(h.panels[0].scrolled, undefined, 'a restore does not scroll'); | |
| 188 | + | assert.equal(h.panels[1].scrolled, undefined); | |
| 189 | + | location.hash = ''; | |
| 185 | 190 | window.ermiteMR.panels(-1); | |
| 186 | 191 | assert.deepEqual(h.state(), [false, false]); | |
| 187 | 192 | } finally { h.restore(); } | |
| 188 | 193 | }); | |
| 189 | 194 | ||
| 195 | + | test('footer: scripts take over the panels; a fragment opens and scrolls to its panel', () => { | |
| 196 | + | const h = footerHarness(); | |
| 197 | + | try { | |
| 198 | + | location.hash = '#mr-checks'; | |
| 199 | + | window.ermiteMR.panels(); | |
| 200 | + | assert.equal(h.footer.attrs['data-panels'], '', 'the CSS fallback of the fragment stops'); | |
| 201 | + | assert.deepEqual(h.state(), [false, true]); | |
| 202 | + | assert.equal(h.panels[1].scrolled, 1, 'the panel of the fragment is brought in'); | |
| 203 | + | location.hash = '#mr-commits'; h.listeners.hashchange(); | |
| 204 | + | assert.deepEqual(h.state(), [true, false]); | |
| 205 | + | assert.equal(h.panels[0].scrolled, 1); | |
| 206 | + | } finally { h.restore(); } | |
| 207 | + | }); | |
| 208 | + | ||
| 190 | 209 | test('touch range: first tap anchors, second tap opens the sorted range', () => { | |
| 191 | 210 | const base = 'https://forge.test/demo/merge-requests/1'; | |
| 192 | 211 | const first = base + '?on=new:24:a.go#f1'; | |
| @@ -97,7 +97,8 @@ function footerHarness(extra = {}) { | |||||
| 97 | const saved = Object.fromEntries(['document', 'location', 'addEventListener', 'history', 'MutationObserver'].map((k) => [k, globalThis[k]])); | 97 | const saved = Object.fromEntries(['document', 'location', 'addEventListener', 'history', 'MutationObserver'].map((k) => [k, globalThis[k]])); | ||
| 98 | const node = (id) => ({ id, attrs: {}, events: {}, hidden: false, | 98 | const node = (id) => ({ id, attrs: {}, events: {}, hidden: false, | ||
| 99 | setAttribute(k, v) { this.attrs[k] = v; }, getAttribute(k) { return this.attrs[k] ?? null; }, | 99 | setAttribute(k, v) { this.attrs[k] = v; }, getAttribute(k) { return this.attrs[k] ?? null; }, | ||
| 100 | - | addEventListener(k, v) { this.events[k] = v; }, focus() { this.focused = true; } }); | 100 | + | addEventListener(k, v) { this.events[k] = v; }, focus() { this.focused = true; }, |
| 101 | + | scrollIntoView() { this.scrolled = (this.scrolled ?? 0) + 1; } }); | |||
| 101 | const tabs = [node('mr-tab-commits'), node('mr-tab-checks')]; | 102 | const tabs = [node('mr-tab-commits'), node('mr-tab-checks')]; | ||
| 102 | const panels = [node('mr-commits'), node('mr-checks-panel')]; | 103 | const panels = [node('mr-commits'), node('mr-checks-panel')]; | ||
| 103 | const footer = node('footer'); footer.querySelectorAll = () => panels; | 104 | const footer = node('footer'); footer.querySelectorAll = () => panels; | ||
| @@ -110,7 +111,7 @@ function footerHarness(extra = {}) { | |||||
| 110 | globalThis.history = { replaceState: (...args) => replaced.push(args) }; | 111 | globalThis.history = { replaceState: (...args) => replaced.push(args) }; | ||
| 111 | const restore = () => { for (const [k, v] of Object.entries(saved)) { if (v === undefined) delete globalThis[k]; else globalThis[k] = v; } }; | 112 | const restore = () => { for (const [k, v] of Object.entries(saved)) { if (v === undefined) delete globalThis[k]; else globalThis[k] = v; } }; | ||
| 112 | const state = () => panels.map((p) => !p.hidden); | 113 | const state = () => panels.map((p) => !p.hidden); | ||
| 113 | - | return { tabs, panels, listeners, replaced, restore, state }; | 114 | + | return { tabs, panels, footer, listeners, replaced, restore, state }; |
| 114 | } | 115 | } | ||
| 115 | 116 | ||||
| 116 | test('footer tabs: closed by default, toggle, switch, arrows, hash', () => { | 117 | test('footer tabs: closed by default, toggle, switch, arrows, hash', () => { | ||
| @@ -178,15 +179,33 @@ test('footer: #mr-ck syncs the checks label; its click opens Checks without togg | |||||
| 178 | test('footer: panels(index) restores the open panel without touching the URL', () => { | 179 | test('footer: panels(index) restores the open panel without touching the URL', () => { | ||
| 179 | const h = footerHarness(); | 180 | const h = footerHarness(); | ||
| 180 | try { | 181 | try { | ||
| 182 | + | location.hash = '#mr-commits'; | |||
| 181 | window.ermiteMR.panels(1); | 183 | window.ermiteMR.panels(1); | ||
| 182 | - | assert.deepEqual(h.state(), [false, true]); | 184 | + | assert.deepEqual(h.state(), [false, true], 'a restore ignores the fragment'); |
| 183 | assert.equal(h.tabs[1].attrs['aria-expanded'], 'true'); | 185 | assert.equal(h.tabs[1].attrs['aria-expanded'], 'true'); | ||
| 184 | assert.deepEqual(h.replaced, []); | 186 | assert.deepEqual(h.replaced, []); | ||
| 187 | + | assert.equal(h.panels[0].scrolled, undefined, 'a restore does not scroll'); | |||
| 188 | + | assert.equal(h.panels[1].scrolled, undefined); | |||
| 189 | + | location.hash = ''; | |||
| 185 | window.ermiteMR.panels(-1); | 190 | window.ermiteMR.panels(-1); | ||
| 186 | assert.deepEqual(h.state(), [false, false]); | 191 | assert.deepEqual(h.state(), [false, false]); | ||
| 187 | } finally { h.restore(); } | 192 | } finally { h.restore(); } | ||
| 188 | }); | 193 | }); | ||
| 189 | 194 | ||||
| 195 | + | test('footer: scripts take over the panels; a fragment opens and scrolls to its panel', () => { | |||
| 196 | + | const h = footerHarness(); | |||
| 197 | + | try { | |||
| 198 | + | location.hash = '#mr-checks'; | |||
| 199 | + | window.ermiteMR.panels(); | |||
| 200 | + | assert.equal(h.footer.attrs['data-panels'], '', 'the CSS fallback of the fragment stops'); | |||
| 201 | + | assert.deepEqual(h.state(), [false, true]); | |||
| 202 | + | assert.equal(h.panels[1].scrolled, 1, 'the panel of the fragment is brought in'); | |||
| 203 | + | location.hash = '#mr-commits'; h.listeners.hashchange(); | |||
| 204 | + | assert.deepEqual(h.state(), [true, false]); | |||
| 205 | + | assert.equal(h.panels[0].scrolled, 1); | |||
| 206 | + | } finally { h.restore(); } | |||
| 207 | + | }); | |||
| 208 | + | ||||
| 190 | test('touch range: first tap anchors, second tap opens the sorted range', () => { | 209 | test('touch range: first tap anchors, second tap opens the sorted range', () => { | ||
| 191 | const base = 'https://forge.test/demo/merge-requests/1'; | 210 | const base = 'https://forge.test/demo/merge-requests/1'; | ||
| 192 | const first = base + '?on=new:24:a.go#f1'; | 211 | const first = base + '?on=new:24:a.go#f1'; | ||
M internal/adapters/web/mr_test.go +3 -0
| @@ -142,6 +142,9 @@ func TestMRPanelsAndMenus(t *testing.T) { | |||
| 142 | 142 | assert.Contains(t, footer, `id="mr-commits" data-mr-panel role="region"`) | |
| 143 | 143 | assert.Contains(t, footer, `id="mr-checks-panel" data-mr-panel role="region"`) | |
| 144 | 144 | assert.NotContains(t, footer, `data-mr-panel hidden`) | |
| 145 | + | css, err := staticFS.ReadFile("static/css/60-mr.css") | |
| 146 | + | require.NoError(t, err) | |
| 147 | + | assert.Contains(t, string(css), `&:not([data-panels]) [data-mr-panel]:not(:target, :has(:target)) { display: none; }`) | |
| 145 | 148 | assert.Contains(t, footer, `data-mr-tabs`) | |
| 146 | 149 | assert.Contains(t, footer, `id="mr-commits"`) | |
| 147 | 150 | assert.Contains(t, footer, `id="mr-checks"`) | |
| @@ -142,6 +142,9 @@ func TestMRPanelsAndMenus(t *testing.T) { | |||||
| 142 | assert.Contains(t, footer, `id="mr-commits" data-mr-panel role="region"`) | 142 | assert.Contains(t, footer, `id="mr-commits" data-mr-panel role="region"`) | ||
| 143 | assert.Contains(t, footer, `id="mr-checks-panel" data-mr-panel role="region"`) | 143 | assert.Contains(t, footer, `id="mr-checks-panel" data-mr-panel role="region"`) | ||
| 144 | assert.NotContains(t, footer, `data-mr-panel hidden`) | 144 | assert.NotContains(t, footer, `data-mr-panel hidden`) | ||
| 145 | + | css, err := staticFS.ReadFile("static/css/60-mr.css") | |||
| 146 | + | require.NoError(t, err) | |||
| 147 | + | assert.Contains(t, string(css), `&:not([data-panels]) [data-mr-panel]:not(:target, :has(:target)) { display: none; }`) | |||
| 145 | assert.Contains(t, footer, `data-mr-tabs`) | 148 | assert.Contains(t, footer, `data-mr-tabs`) | ||
| 146 | assert.Contains(t, footer, `id="mr-commits"`) | 149 | assert.Contains(t, footer, `id="mr-commits"`) | ||
| 147 | assert.Contains(t, footer, `id="mr-checks"`) | 150 | assert.Contains(t, footer, `id="mr-checks"`) | ||
M internal/adapters/web/static/css/60-mr.css +1 -2
| @@ -25,8 +25,7 @@ | |||
| 25 | 25 | & [data-mr-tabs] .count { margin-left: 0; color: var(--ink-hi); background: var(--bg-canvas); box-shadow: inset 0 0 0 1px var(--border); } | |
| 26 | 26 | & [data-mr-tabs] a:hover { background: var(--bg-strong); color: var(--ink-hi); } | |
| 27 | 27 | & [data-mr-tabs] a[aria-expanded="true"] { border-color: var(--teal); color: var(--teal); background: var(--bg-surface); } | |
| 28 | - | & [data-mr-panel] { padding-block-start: var(--s2); } | |
| 29 | - | & [data-mr-panel] { overflow-x: auto; } | |
| 28 | + | & [data-mr-panel] { padding-block-start: var(--s2); overflow-x: auto; } | |
| 30 | 29 | & [hidden] { display: none; } | |
| 31 | 30 | /* Until mr.js drives the panels (data-panels, then [hidden]), only the | |
| 32 | 31 | panel of the URL fragment shows: the tabs work as anchors without | |
| @@ -25,8 +25,7 @@ | |||||
| 25 | & [data-mr-tabs] .count { margin-left: 0; color: var(--ink-hi); background: var(--bg-canvas); box-shadow: inset 0 0 0 1px var(--border); } | 25 | & [data-mr-tabs] .count { margin-left: 0; color: var(--ink-hi); background: var(--bg-canvas); box-shadow: inset 0 0 0 1px var(--border); } | ||
| 26 | & [data-mr-tabs] a:hover { background: var(--bg-strong); color: var(--ink-hi); } | 26 | & [data-mr-tabs] a:hover { background: var(--bg-strong); color: var(--ink-hi); } | ||
| 27 | & [data-mr-tabs] a[aria-expanded="true"] { border-color: var(--teal); color: var(--teal); background: var(--bg-surface); } | 27 | & [data-mr-tabs] a[aria-expanded="true"] { border-color: var(--teal); color: var(--teal); background: var(--bg-surface); } | ||
| 28 | - | & [data-mr-panel] { padding-block-start: var(--s2); } | 28 | + | & [data-mr-panel] { padding-block-start: var(--s2); overflow-x: auto; } |
| 29 | - | & [data-mr-panel] { overflow-x: auto; } | |||
| 30 | & [hidden] { display: none; } | 29 | & [hidden] { display: none; } | ||
| 31 | /* Until mr.js drives the panels (data-panels, then [hidden]), only the | 30 | /* Until mr.js drives the panels (data-panels, then [hidden]), only the | ||
| 32 | panel of the URL fragment shows: the tabs work as anchors without | 31 | panel of the URL fragment shows: the tabs work as anchors without | ||
M internal/adapters/web/static/js/mr.js +7 -0
| @@ -600,6 +600,13 @@ window.ermiteMR = (() => { | |||
| 600 | 600 | for (const f of $$('#diffs details.dfile', page)) if (files.has(f.dataset.file)) f.open = files.get(f.dataset.file); | |
| 601 | 601 | ++diffGeneration; ++revealGen; moved = false; loading.clear(); lazyIO?.disconnect(); | |
| 602 | 602 | $('#diffs', page).dataset.mrStarted = 'true'; | |
| 603 | + | // The fresh footer comes in with the panels as they are, so that an open | |
| 604 | + | // panel does not fold and open again while the other regions swap. | |
| 605 | + | const freshFooter = $('.mr-footer', page); | |
| 606 | + | if (freshFooter) { | |
| 607 | + | freshFooter.setAttribute('data-panels', ''); | |
| 608 | + | $$('[data-mr-panel]', freshFooter).forEach((p, i) => { p.hidden = i !== openPanel; }); | |
| 609 | + | } | |
| 603 | 610 | for (const sel of regions) { | |
| 604 | 611 | const fresh = $(sel, page); | |
| 605 | 612 | await replaceRegion($(sel), fresh); | |
| @@ -600,6 +600,13 @@ window.ermiteMR = (() => { | |||||
| 600 | for (const f of $$('#diffs details.dfile', page)) if (files.has(f.dataset.file)) f.open = files.get(f.dataset.file); | 600 | for (const f of $$('#diffs details.dfile', page)) if (files.has(f.dataset.file)) f.open = files.get(f.dataset.file); | ||
| 601 | ++diffGeneration; ++revealGen; moved = false; loading.clear(); lazyIO?.disconnect(); | 601 | ++diffGeneration; ++revealGen; moved = false; loading.clear(); lazyIO?.disconnect(); | ||
| 602 | $('#diffs', page).dataset.mrStarted = 'true'; | 602 | $('#diffs', page).dataset.mrStarted = 'true'; | ||
| 603 | + | // The fresh footer comes in with the panels as they are, so that an open | |||
| 604 | + | // panel does not fold and open again while the other regions swap. | |||
| 605 | + | const freshFooter = $('.mr-footer', page); | |||
| 606 | + | if (freshFooter) { | |||
| 607 | + | freshFooter.setAttribute('data-panels', ''); | |||
| 608 | + | $$('[data-mr-panel]', freshFooter).forEach((p, i) => { p.hidden = i !== openPanel; }); | |||
| 609 | + | } | |||
| 603 | for (const sel of regions) { | 610 | for (const sel of regions) { | ||
| 604 | const fresh = $(sel, page); | 611 | const fresh = $(sel, page); | ||
| 605 | await replaceRegion($(sel), fresh); | 612 | await replaceRegion($(sel), fresh); | ||