You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "andotorg (via GitHub)" <gi...@apache.org> on 2026/08/29 09:56:06 UTC
[PR] Remove Titlebar top blank [maka]
andotorg opened a new pull request, #4153:
URL: https://github.com/apache/maka/pull/4153
## remove Titlebar top blank
## Summary
Module pages (Extensions, Scheduled tasks, etc.) showed a large empty band under the frameless titlebar. `.mainColumn` already reserves `--maka-plate-titlebar-clearance`, and `.maka-module-main[data-page-shell="layout"]` also applied `padding-top: var(--space-16)` (64px), so the inset was applied twice.
Remove the extra module-page top padding (`padding: 0`). Titlebar clearance stays solely on `.mainColumn`, and page content aligns to the top of the content plate again.
## Verification
- Visual check: open Extensions / Scheduled tasks — the tall blank above the page header is gone; content still clears the titlebar chrome.
- Did not run full lint / typecheck / test suites (CSS-only change).
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope:
Cursor Agent — diagnosed the double titlebar inset and updated `module-shell.css`.
## Checklist
- [ ] Tests cover the change and fail without it
- [ ] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] Remove Titlebar top blank [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4153:
URL: https://github.com/apache/maka/pull/4153
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] Remove Titlebar top blank [maka]
Posted by "YayoiNanoka (via GitHub)" <gi...@apache.org>.
YayoiNanoka commented on PR #4153:
URL: https://github.com/apache/maka/pull/4153#issuecomment-5468405222
## PR Assessment
### What problem does this PR solve?
Module pages receive two independent top insets:
- `.mainColumn` reserves 32px for the frameless titlebar.
- `.maka-module-main[data-page-shell="layout"]` adds another 64px.
This places module-page content 96px below the plate top, producing the reported empty band.
### How does this PR solve the problem?
The PR changes the shared module shell padding from `var(--space-16) 0 0` to `0`, leaving `.mainColumn` as the sole owner of titlebar clearance. It affects Skills, MCP, Scheduled tasks, and Daily review through their shared shell and adds an explanatory ownership comment.
No unrelated behavior or state is changed.
### Is the problem correctly defined?
Correct.
The actual CSS cascade supports the stated root cause: the titlebar inset is duplicated rather than missing or incorrectly calculated.
### Principle-based assessment
- First principles: titlebar clearance should have one owner. `.mainColumn` already provides the required 32px clearance to every view it contains.
- Occam's razor: removing the redundant declaration is sufficient; no new state, selector, or abstraction is introduced.
- Minimal sufficient solution: the behavioral change is one CSS declaration, scoped to the existing shared module shell.
## Review Findings
### P1 Blocker
None.
### P2 Should Fix
None.
### Suggestion
None.
## Verification
- PR merge base: `b16376d782a2c7d3540a67708851f9256d87dde6`
- Current `main` integration base: `be3a0291b5401ea23580062113bf9e0d20e6ca0f`
- Head: `51bfdf3269231428e2ecc35140bca5514e66463e`
- Diff range: `b16376d782a2c7d3540a67708851f9256d87dde6...51bfdf3269231428e2ecc35140bca5514e66463e`
- Executed:
- Fetched and verified the exact PR head in an isolated clone with push disabled.
- Verified a clean merge into current `main`; resulting tree: `dac444ba50cdef0e96524a11b44a6318fa1e7475`.
- Ran `git diff --check`.
- Ran Biome against the changed CSS file.
- Built the complete production CSS import graph for both base and head.
- Loaded both compiled CSS graphs in headless Chrome using the relevant application shell structure.
- Key results:
- Base: `.mainColumn` padding = 32px, module padding = 64px, page content begins at y=96.
- Head: `.mainColumn` padding = 32px, module padding = 0px, page content begins at y=32.
- The PR removes exactly the duplicate 64px inset while preserving titlebar clearance.
- No selector-related drift was found between the PR merge base and current `main`.
- Biome and CSS graph builds passed.
- Temporary tests: removed; never committed or pushed. The isolated review directory was removed.
- Original workspace: clean at review start, but seven unrelated tracked files changed concurrently during the review. No review command targeted those paths, and they were not inspected or altered; unchanged status cannot therefore be confirmed.
- Not verified:
- A real Electron-window visual pass of every module page.
- Full lint, typecheck, and test suites.
- Full renderer build was inconclusive because reused workspace build artifacts exposed an `@maka/core` export surface that did not match the isolated PR checkout; this was not attributed to the PR.
- Hosted CI currently exposes only a successful `label` check.
## Conclusion
Can merge
The CSS ownership model, compiled cascade, measured base/head behavior, and clean integration with current `main` all support the change. No P1 or P2 issue was found.
<details>
<summary>点击展开中文</summary>
## PR 判断
### 这个 PR 解决了什么问题？
模块页面同时获得了两层独立的顶部留白：
- `.mainColumn` 为无边框标题栏预留了 32px。
- `.maka-module-main[data-page-shell="layout"]` 又增加了 64px。
因此模块页内容从 plate 顶部向下偏移了 96px，形成所报告的大块空白区域。
### 这个 PR 如何解决这个问题？
PR 将共享模块 shell 的 padding 从 `var(--space-16) 0 0` 改为 `0`，让 `.mainColumn` 成为标题栏 clearance 的唯一所有者。Skills、MCP、定时任务和每日回顾都通过共享 shell 获得这一修改；同时增加了说明该所有权的注释。
没有修改无关行为或状态。
### 这个问题定义得对吗？
Correct（正确）。
实际 CSS 级联支持 PR 所述的根因：标题栏 inset 被重复应用，而不是缺失或计算错误。
### 原则性判断
- 第一性原理：标题栏 clearance 应只有一个所有者；`.mainColumn` 已经为其中的所有页面提供了所需的 32px clearance。
- 奥卡姆剃刀：删除冗余声明已经足够，不需要引入新状态、新选择器或新抽象。
- 最小充分解：行为改动只有一条 CSS 声明，并且限定在既有的共享模块 shell 中。
## Review Findings
### P1 阻塞
无。
### P2 应该改
无。
### 建议
无。
## 验证
- PR merge base：`b16376d782a2c7d3540a67708851f9256d87dde6`
- 当前 `main` 集成基线：`be3a0291b5401ea23580062113bf9e0d20e6ca0f`
- Head：`51bfdf3269231428e2ecc35140bca5514e66463e`
- Diff 范围：`b16376d782a2c7d3540a67708851f9256d87dde6...51bfdf3269231428e2ecc35140bca5514e66463e`
- 已执行：
- 在禁用 push 的隔离 clone 中获取并核对准确的 PR head。
- 验证可以干净合入当前 `main`；结果 tree 为 `dac444ba50cdef0e96524a11b44a6318fa1e7475`。
- 执行 `git diff --check`。
- 对修改的 CSS 文件执行 Biome 检查。
- 分别为 base 和 head 构建完整的生产 CSS import graph。
- 使用相关应用 shell 结构，在无头 Chrome 中加载两份编译后的 CSS。
- 关键结果：
- Base：`.mainColumn` padding 为 32px，模块 padding 为 64px，页面内容从 y=96 开始。
- Head：`.mainColumn` padding 为 32px，模块 padding 为 0px，页面内容从 y=32 开始。
- PR 精确删除了重复的 64px inset，同时保留了标题栏 clearance。
- PR merge base 到当前 `main` 之间没有发现与相关选择器有关的漂移。
- Biome 与 CSS graph 构建均通过。
- 临时测试：已删除，从未 commit 或 push；隔离审查目录也已删除。
- 原工作区：审查开始时为 clean，但审查期间有 7 个无关的已跟踪文件并发发生修改。审查命令没有以这些路径为目标，也没有检查或处理它们，因此无法确认原工作区全程未变。
- 未验证：
- 在真实 Electron 窗口中逐一目测所有模块页。
- 完整 lint、typecheck 和测试套件。
- 完整 renderer 构建因复用的工作区构建产物所暴露的 `@maka/core` export surface 与隔离 PR checkout 不匹配而无法得出结论；未将其归因于本 PR。
- 托管 CI 当前只显示一个成功的 `label` 检查。
## 结论
可以合入
CSS 所有权模型、编译后的级联、base/head 实测行为以及与当前 `main` 的干净集成都支持这项修改。没有发现 P1 或 P2 问题。
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org