-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(web-shell): realign two suites with shipped behavior to unbreak main CI (#11525) #11530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+17
−1
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] The new
beforeEachfixture hardcodes the sidebar-width storage key as a raw literal — a fifth copy ofSIDEBAR_WIDTH_STORAGE_KEY, whichWebShellSidebar.tsx:149does not export — and the two tooltip assertions that depend on it carry no assertion message. Together they make the fixture's layout precondition invisible at the point of failure.If
SIDEBAR_FOOTER_COMPACT_WIDTH(currently344,WebShellSidebar.tsx:154) is raised above 360, if the storage key is renamed, or if the sidebar stops reading its persisted width at mount,readSidebarWidth()falls back toSIDEBAR_DEFAULT_WIDTH = 260, the version label leaves the footer row, and bothtreats an empty name as unset rather than blanking the brand rowandnames the version tooltip after the brandfail withexpected null not to be null. Nothing in that output mentions the sidebar width, the storage key or the footer breakpoint, so the natural read is a branding regression and the investigation goes intobrandContextand the footer version badge instead of into this fixture.Witness:
The two failures are exactly the two this fixture feeds, and the printed output nowhere names the width, the key or the breakpoint. The sibling suite corroborates the mechanism unmodified:
WebShellSidebar.footer-version.test.tsxmounts at 260 and asserts the version badge is null, then mounts at 360 and asserts it is non-null.The fix spans three places, so it is written out rather than offered as a one-click suggestion: declare the key and the width as named locals at module scope (mirroring
WebShellSidebar.footer-version.test.tsx:127and itsmountAtWidthhelper), use the local in thissetItem, and attach that sibling file's diagnostic-message idiom to the two tooltip queries.Any width chosen here must stay above the breakpoint and inside the clamp:
SIDEBAR_FOOTER_COMPACT_WIDTH = 344(WebShellSidebar.tsx:154), clamped byMath.min(getSidebarMaxWidth(), Math.max(SIDEBAR_MIN_WIDTH, width))(WebShellSidebar.tsx:593-594) withSIDEBAR_MIN_WIDTH = 220/SIDEBAR_MAX_WIDTH = 420(WebShellSidebar.tsx:151-152) — keep360, because the260default does not clear344.中文说明
新增的
beforeEachfixture 把侧边栏宽度的 storage key 硬编码为裸字面量 —— 这是SIDEBAR_WIDTH_STORAGE_KEY的第五份拷贝,而WebShellSidebar.tsx:149并未导出该常量 —— 同时依赖它的两条 tooltip 断言都没有断言消息。两者合起来,使这个 fixture 的布局前提在失败点上完全不可见。如果
SIDEBAR_FOOTER_COMPACT_WIDTH(当前为344,WebShellSidebar.tsx:154)被提高到 360 以上、如果该 storage key 被重命名、或者侧边栏不再在挂载时读取持久化宽度,readSidebarWidth()就会回退到SIDEBAR_DEFAULT_WIDTH = 260,版本标签移出行,于是treats an empty name as unset rather than blanking the brand row与names the version tooltip after the brand两个用例都会以expected null not to be null失败。该输出中没有任何地方提到侧边栏宽度、storage key 或页脚断点,因此最自然的理解是品牌配置出现了回归,排查会走向brandContext和页脚版本徽章,而不是这个 fixture。证据:
这两个失败正是该 fixture 所支撑的两个用例,而输出中完全没有出现宽度、key 或断点。姊妹测试套件在未改动的情况下印证了同一机制:
WebShellSidebar.footer-version.test.tsx在 260 下挂载并断言版本徽章为 null,在 360 下挂载并断言其非 null。修复涉及三处,因此以代码块列出而不是一键 suggestion:在模块作用域把 key 与宽度声明为具名局部常量(参照
WebShellSidebar.footer-version.test.tsx:127及其mountAtWidth辅助函数),在此setItem中使用该常量,并为两条 tooltip 查询加上该姊妹文件的诊断消息写法。此处选择的宽度必须保持在断点之上且落在 clamp 范围内:
SIDEBAR_FOOTER_COMPACT_WIDTH = 344(WebShellSidebar.tsx:154),经Math.min(getSidebarMaxWidth(), Math.max(SIDEBAR_MIN_WIDTH, width))(WebShellSidebar.tsx:593-594)裁剪,其中SIDEBAR_MIN_WIDTH = 220/SIDEBAR_MAX_WIDTH = 420(WebShellSidebar.tsx:151-152)—— 请保留360,因为默认值260无法越过344。— qwen3.8-max via Qwen Code /review (v0.23.2)