Fix the WYSIWYG <Tabs> strip showing phantom tab pills when a tab…#248
Merged
Conversation
* fix(open-knowledge): scope Tabs strip slot walk to direct children The WYSIWYG Tabs strip rendered a phantom pill for every component nested inside a Tab. A Tab is itself a ProseMirror container with its own contentDOM, so readTabSlots' recursive querySelectorAll swept in the .react-renderer of every nested nodeview (a Callout, a Steps and its Steps, a nested Tabs). The quickstart's two install tabs rendered as nine pills, and the spurious pills revealed no panel. Scope the slot walk to the Tabs' own direct children with a :scope chain that mirrors the CSS active-panel reveal in globals.css exactly, so the strip index space and the CSS index space are provably the same set. This removes the old ownership filter and its incomplete nested-Tabs special-case. Unit coverage in Tabs.dom.test.tsx pins readTabSlots over a hand-built nodeview DOM (happy path, single nested container, the quickstart Callout plus Steps shape, and nested Tabs). Browser coverage in tabs-component-strip.e2e.ts asserts the rendered pill count end to end and is wired into the test:e2e subset. * chore(open-knowledge): regenerate ng-anchors catalog testFileCount Sibling merges added test files under the catalog tracked globs without regenerating the committed ng-anchors-catalog.json, so its testFileCount drifted behind the tree (177 committed vs 178 on disk) and the freshness gate failed. Regenerate to realign. No source change; the count reflects tracked files only. * test(open-knowledge): adopt shared provider-sync helper, cover Tabs slot edge cases Address the PR review on the Tabs strip fix: - Replace the inline waitForFunction(isSynced, 15s) in the Tabs component e2e with the shared waitForActiveProviderSynced helper (60s default). The inline 15s was the known-too-aggressive value the helper was created to retire and would flake under workers=4 CI contention. - Add a Tabs.dom test for the non-Tab top-level block fallback (numbered label, null panelId): the index-alignment path the header comment documents as load-bearing was previously untested. - Add a Tabs.dom test for an empty Tabs (zero slots), guarding the :scope chain on a bare contentDOM. * test(open-knowledge): satisfy *.dom.test.tsx RTL value-import contract Tabs.dom.test.tsx carried the .dom.test.tsx suffix but imported only bun:test, violating the precedent #43 filename contract enforced by tests/integration/dom-test-filename-stop-rule.test.ts (every *.dom.test.tsx must value-import from @testing-library/react). That meta-test failed deterministically; CI's one-retry-on-failure then re-ran the whole ~9-min integration suite, pushing it past the 15-min job cap so the job was cancelled rather than reported as a failure. Add the sanctioned cleanup import plus afterEach(cleanup), a no-op here since the tests build DOM via innerHTML, matching tests/dom/source-mode-suggestion-gating.dom.test.tsx. GitOrigin-RevId: b0f6184b44a7b9c2a7a8425c9020f45401529e46
![inkeep-internal-ci[bot]](https://avatars.githubusercontent.com/u/123133689?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated approval from agents-private public-mirror-sync (run: https://github.com/inkeep/agents-private/actions/runs/28095809009). Source of truth is the monorepo; direct edits on inkeep/open-knowledge are overwritten on next sync.
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
1 participant
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.
Fix the WYSIWYG
<Tabs>strip showing phantom tab pills when a tab nests other components. A<Tab>containing a<Steps>,<Callout>, image, or nested<Tabs>used to add one extra empty pill per nested block — the quickstart's two install tabs rendered as eight pills, with the spurious pills revealing no panel. The strip now counts only the Tabs's own direct tab children, matching the CSS active-panel index space exactly.