fix(review): 按独立 review(Chromium 实测)修正 #786 的措辞、守卫与覆盖
reviewer 在本机跑 Chromium 逐条实测(同一批宿主规则 + 插件产物),结论:修复方向与形状正确
(`.mermaidModal` 预修复计算 `no-drag`、修复后 `none`;级联探针确实能把未修复产物判红),
但有几处措辞与守卫不到位。逐条处理:
1. **「app-region 会被继承」的说法在该引擎里不成立**:`[data-dsh-panel-host]` 不是 body
子元素(DOM 是 `body > [data-dsh-better-sidebar] > [data-dsh-panel-host]`),lane 用的
Chromium 不传播该属性(实测:`drag` 父元素里子元素读 `none`)。改写成「宿主是 body
子元素;在 Electron 里该值会传到它内部的面板层(#772 的 CDP 实测),lane 的引擎不传播,
所以面板层自己也声明同一条规则」——CSS 注释、指南 §12.1、探针注释三处同步。
2. **`regions.layer` 那条断言是真空断言**(修复前后都读 `none`):保留为回归钉子,但在
注释里明确标注它不具判别力、面板层的规则由 `panel-host-css.spec.ts` 钉住。
3. **给修复本身补行为级守护**:放大视图已经在同一 lane 里被真实打开过,于是在那里直接对
`[data-mermaid-modal]`(以及它的按钮)读计算值——断言用**页面里宿主自己的** darwin
规则(不再是探针复制的副本)。实测判别力:npm 0.22.0 上 `Received: "no-drag"`、修复版
`none`。
4. **「交互层不要加 initial」与放大视图的矛盾**:放大视图是**遮罩层例外**——模态语义下
只有其中控件必须保持 `no-drag`;残余代价(背景落在顶部拖拽带上时点背景会变拖窗)已写进
指南,避免后来者按字面判据「修」掉其中一边。
5. **`.selectionPopup` 的负向守卫可被选择器列表绕过**(reviewer 实测:`.selectionPopup,.x
{ initial !important }` 能通过):改成按**声明块**扫描——凡带 `-webkit-app-region:
initial` 的块,其选择器文本都不得出现 `selectionPopup`。
6. **补上 `none` 关键字的坑**:字面量 `none` 计算成 `no-drag`(是扣减值),只有 `initial`
/`unset`/`revert` 才是中性值——CSS 注释、指南与单元用例注释都已写明。
7. **未覆盖层给出结论**:`FileTree` 的 `.uploadDropChatHint` 是 body 直挂的装饰层,但只在
OS 文件拖拽悬停期间存在且 `pointer-events:none`,收益极小而**有意不处理**(写进指南,
不再让「覆盖清单」看起来是绝对集合)。
8. 顺带:`§12.2` 的同步集里 `tests/theme.spec.ts` 只守颜色(本轮无颜色改动),形状守护落在
已解析该样式表的 `tests/panel-host-css.spec.ts` 上——指南那句已按此表述。
验证:typecheck / lint 通过;panel-host-css 4 例通过;本地挂载 lane 7 passed(含新的放大
视图计算值断言);对 npm 0.22.0 的判别力实测:宿主探针红(`host` = `no-drag`)、放大视图
断言红(`modal` = `no-drag`),修复版全绿。
This commit is contained in:
+40
-11
@@ -517,6 +517,28 @@ test('plugin mounts into the DSH shell and survives a built-in tab sweep', async
|
||||
const modal = page.locator('[data-mermaid-modal]')
|
||||
await pane.locator('[data-mermaid-diagram] svg').first().click()
|
||||
await expect(modal, 'clicking the diagram must open the zoom modal').toHaveCount(1, { timeout: 10_000 })
|
||||
// While it is up, the modal is the plugin's OTHER viewport-sized BODY CHILD:
|
||||
// without the app-region reset the shell's blanket `body > :not(#root)`
|
||||
// no-drag makes it cancel every window-drag strip (and the macOS
|
||||
// double-click-title zoom) until it closes — issue #772. Measured against the
|
||||
// SHELL's own darwin rules (present in this page's stylesheet, activated by
|
||||
// the platform attribute) rather than a copy: the modal must compute the
|
||||
// neutral `none`, while its toolbar buttons keep `no-drag` so a click on them
|
||||
// is never a window drag.
|
||||
const modalRegions = await page.evaluate(() => {
|
||||
const previous = document.documentElement.dataset.platform
|
||||
document.documentElement.dataset.platform = 'darwin'
|
||||
const modalElement = document.querySelector('[data-mermaid-modal]')
|
||||
const region = (element: Element | null): string => element === null
|
||||
? 'missing'
|
||||
: getComputedStyle(element).getPropertyValue('-webkit-app-region').trim()
|
||||
const result = { modal: region(modalElement), button: region(modalElement?.querySelector('button') ?? null) }
|
||||
if (previous === undefined) delete document.documentElement.dataset.platform
|
||||
else document.documentElement.dataset.platform = previous
|
||||
return result
|
||||
})
|
||||
expect(modalRegions.modal, 'the zoom modal must opt out of app-region computation').toBe('none')
|
||||
expect(modalRegions.button, 'its buttons must stay no-drag').toBe('no-drag')
|
||||
await page.keyboard.press('Escape')
|
||||
await expect(modal, 'Esc must close the zoom modal').toHaveCount(0, { timeout: 10_000 })
|
||||
await assertNoCrash()
|
||||
@@ -615,15 +637,17 @@ test('conservative auto: URL stamps alone never modify the layout; plugin chrome
|
||||
return false
|
||||
})
|
||||
expect(hasNoDragRule, 'the bundle must ship the drag-region opt-out rule').toBe(true)
|
||||
// The viewport-sized panel host must NOT inherit the shell's blanket
|
||||
// `body > :not(#root) { no-drag }` — it would cancel every window-drag
|
||||
// strip beneath it on macOS (issue #772). A text match only proves the
|
||||
// declaration shipped, so this probe reproduces the SHELL's two darwin
|
||||
// rules verbatim, flips the platform attribute the shell stamps, and reads
|
||||
// the COMPUTED property: app-region is inherited and ignores
|
||||
// pointer-events, so the loss shows on the host/layer — never on the strip
|
||||
// itself (it computes `drag` in every configuration, which is why a fix
|
||||
// validated only against the strip proves nothing).
|
||||
// The plugin host is a direct body child, so on macOS it picks up the
|
||||
// shell's blanket `body > :not(#root) { no-drag }` and — because app-region
|
||||
// ignores pointer-events — the viewport-sized layer inside it cancelled every
|
||||
// window-drag strip beneath it (issue #772). A text match only proves the
|
||||
// declaration shipped, so this probe reproduces the SHELL's two darwin rules
|
||||
// verbatim, flips the platform attribute the shell stamps, and reads the
|
||||
// COMPUTED property. Two things to know when reading the expectations below:
|
||||
// the strip itself computes `drag` in every configuration (a fix validated
|
||||
// against the strip alone proves nothing), and `[data-dsh-panel-host]` is NOT
|
||||
// a body child (it lives inside the host), so its own value is pinned by the
|
||||
// unit guard rather than by this probe — see the `layer` expectation.
|
||||
const regions = await page.evaluate(() => {
|
||||
const style = document.createElement('style')
|
||||
style.textContent = [
|
||||
@@ -660,8 +684,13 @@ test('conservative auto: URL stamps alone never modify the layout; plugin chrome
|
||||
return result
|
||||
})
|
||||
expect(regions.strip, 'the drag strip itself must keep computing `drag`').toBe('drag')
|
||||
expect(regions.host, 'the plugin host must not inherit the shell no-drag').toBe('none')
|
||||
expect(regions.layer, 'the panel host must not inherit the shell no-drag').toBe('none')
|
||||
expect(regions.host, 'the plugin host must not take the shell blanket no-drag').toBe('none')
|
||||
// NOT discriminating here, and kept as a regression nail only: the layer is
|
||||
// not a body child, and this engine does not propagate the property into it,
|
||||
// so it reads `none` pre- and post-fix. Its rule is pinned by
|
||||
// tests/panel-host-css.spec.ts; in Electron (#772's CDP reading) the layer
|
||||
// DOES end up no-drag through that propagation.
|
||||
expect(regions.layer, 'the panel layer must keep its own reset').toBe('none')
|
||||
expect(regions.layerChild, 'panels stay no-drag so their controls keep receiving clicks').toBe('no-drag')
|
||||
})
|
||||
|
||||
|
||||
@@ -49,12 +49,14 @@ describe('panel host layer css', () => {
|
||||
// The shell's `html[data-platform=darwin] body > :not(#root) { no-drag }`
|
||||
// carries an id in its specificity, so only `initial !important` wins;
|
||||
// `initial` computes to the NEUTRAL `none` (does not subtract from the
|
||||
// drag region) while `no-drag` is the subtracting value — hence the
|
||||
// split: decorative viewport-sized layers opt out, their panels stay
|
||||
// no-drag so controls keep receiving clicks. Dropping the `!important`
|
||||
// silently re-breaks window drag / double-click-title zoom on macOS
|
||||
// (there is no macOS runner to catch it), so pin the shape here where
|
||||
// Linux `pnpm test` fails instead.
|
||||
// drag region) while `no-drag` is the subtracting value — and the literal
|
||||
// keyword `none` is NOT neutral (it computes to `no-drag`), which is
|
||||
// exactly why this guard pins `initial`. Hence the split: decorative
|
||||
// viewport-sized layers opt out, their panels stay no-drag so controls
|
||||
// keep receiving clicks. Dropping the `!important` silently re-breaks
|
||||
// window drag / double-click-title zoom on macOS (there is no macOS
|
||||
// runner to catch it), so pin the shape here where Linux `pnpm test`
|
||||
// fails instead.
|
||||
const text = stripComments(css)
|
||||
expect(text, 'the panel host must opt out of app-region computation').toMatch(
|
||||
/:global\(\[data-dsh-panel-host\]\)\s*\{[^}]*-webkit-app-region:\s*initial\s*!important/,
|
||||
@@ -62,12 +64,21 @@ describe('panel host layer css', () => {
|
||||
expect(text, 'panels must stay no-drag').toMatch(
|
||||
/:global\(\[data-dsh-panel-host\]\)\s*>\s*\*\s*\{[^}]*-webkit-app-region:\s*no-drag/,
|
||||
)
|
||||
expect(text, 'the mermaid zoom modal is the other viewport-sized body child').toMatch(
|
||||
expect(text, 'the mermaid zoom modal is the other persistent viewport-sized body child').toMatch(
|
||||
/\.mermaidModal\s*\{[^}]*-webkit-app-region:\s*initial\s*!important/,
|
||||
)
|
||||
// The interactive popups must NOT get `initial`: they are buttons/handles
|
||||
// the shell already keeps no-drag, and outranking that rule makes a press
|
||||
// start a window drag (the #103/#111 click-swallow class).
|
||||
expect(text).not.toMatch(/\.selectionPopup\s*\{[^}]*-webkit-app-region:\s*initial/)
|
||||
// start a window drag (the #103/#111 click-swallow class). Checked per
|
||||
// DECLARATION BLOCK, not per single-selector rule: a selector list
|
||||
// (`.selectionPopup, .other { … initial !important }`) would slip past a
|
||||
// simple "class immediately followed by {" pattern.
|
||||
const resets = [...text.matchAll(/([^{}]+)\{([^{}]*)\}/g)]
|
||||
.filter(([, , body]) => /-webkit-app-region:\s*initial/.test(body ?? ''))
|
||||
expect(resets.length, 'the file must declare the app-region opt-out').toBeGreaterThan(0)
|
||||
const resetSelectors = resets.map(([, selector]) => selector ?? '').join(',')
|
||||
for (const excluded of ['selectionPopup']) {
|
||||
expect(resetSelectors, `${excluded} must not opt out of app-region`).not.toContain(excluded)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user