Conversation
#505 turned the new-tab picker's category switcher into a tablist (Channels, DMs, Tools). The crowded-tab journey still read every role=tab in the workspace, so with the picker open it counted 2 panel tabs plus 3 picker tabs and failed at channel-tabs.spec.mjs:309 on Chromium and WebKit. Its last()/first() lookups also resolved to picker tabs instead of the strip. Scope those lookups to the "Panel tabs" tablist, matching how #505 scoped the other counts in this file. Product behavior is unchanged. Co-authored-by: classy-murderbot <noreply@buzz.local> Signed-off-by: Logan Johnson <loganj@squareup.com>
Contributor
Author
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Fixes the deterministic
channel-tabs.spec.mjs:309failure on main (Chromium and WebKit):expected 2 tabs, received 5.Failing run: https://github.com/block/buzz-app/actions/runs/36919997681/job/110564390755 (main
69a9af23)Cause
#505 (
e697aa7a, "Use top tabs in the new-tab picker") replaced the picker's category buttons with aTabstablist (Channels, DMs, Tools). The crowded-tab journey opens the picker and countsworkspace.getByRole("tab"), so it got 2 panel tabs plus 3 picker category tabs. #505 scoped the other two counts in this file to the "Panel tabs" tablist but missed this test, which was in a different shard.The last main run with green browser journeys was at
999dbde0.e697aa7ais the next commit, and every main run since has failed this test the same way.The product change was intended. The test's selector was stale, so this fixes the test only.
Fix
In that test, every tab lookup now goes through the existing
listlocator (the "Panel tabs" tablist). Besides the count at 309, this also fixes thefirst()/last()lookups for position and Home/End focus. Those had been resolving to the picker's tabs instead of the panel strip. No assertions were removed or loosened.Relationship to #514
#514 (virtualized thread replies) doesn't touch channel tabs or the picker. It fails this test only because its base includes #505. It should go green once this lands and it rebases.
Validation
Local, macOS, headless, at
4ef0d802on69a9af23:channel-tabs.spec.mjs:281fails on chromium and webkit at line 309 with the same error.playwright test channel-tabs.spec.mjs:281 --project chromium --project webkit --repeat-each 4: 8 passed.playwright test channel-tabs.spec.mjs todos.spec.mjs --project chromium --project webkit: 18 passed.vitest run src/bundled/channels: 464 passed.tsc --noEmit: pass.biome check --error-on-warningson the changed file: pass.