fix(docs): add queryString to tabs#8429
Merged
fatih-acar merged 2 commits intostablefrom Feb 27, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (24)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (14)
WalkthroughThis pull request updates many documentation files to add the 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
type/documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/docs/guides/object-conversion.mdx (1)
235-235:⚠️ Potential issue | 🟡 MinorRemove stray git command from documentation content
Line 235 contains a bare shell command (
git checkout -b atg-20251020-infp-258) that is outside any code block and will be rendered as visible plain text on the documentation page. This is an accidental insertion that must be removed before merge.🛠 Proposed fix
- git checkout -b atg-20251020-infp-258🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/guides/object-conversion.mdx` at line 235, Remove the stray shell command "git checkout -b atg-20251020-infp-258" from the documentation content so it is not rendered as plain text; locate the occurrence in docs/guides/object-conversion.mdx (the literal line containing that command) and delete it or move it into a proper fenced code block if the checkout example is intentional, ensuring the surrounding markdown remains valid.docs/docs/guides/resource-manager.mdx (1)
153-162:⚠️ Potential issue | 🟡 MinorPre-existing: unclosed
:::successadmonition wraps section content including the first changed TabsThe
:::successblock opened at line 153 is never closed with a corresponding:::before the### Creating an IP Prefix objectheading (line 157) and the Tabs component (line 162). In Docusaurus MDX, an unclosed admonition swallows all following content until the next:::marker — meaning the section heading, descriptive text, and the Tabs block at line 162 are all rendered inside the success callout. This is a pre-existing structural defect (not introduced by this PR) but it affects presentation of the section updated here.✏️ Proposed fix
:::success You should see: `schema 'schema.yml' loaded successfully` +::: + A `CoreIPAddressPool` allows you to dynamically allocate IP addresses from one or multiple source IP prefixes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/guides/resource-manager.mdx` around lines 153 - 162, The file opens an admonition with :::success that is never closed, causing the "### Creating an IP Prefix object" heading and the <Tabs groupId="method" queryString> block to be included inside the callout; fix by adding a closing ::: immediately before the "### Creating an IP Prefix object" heading (i.e., close the :::success block) so the admonition ends before the heading and the Tabs component.
🧹 Nitpick comments (3)
docs/src/theme/Tabs/index.tsx (2)
16-26: Overly restrictive guard:props.groupIdnot required whenqueryStringis an explicit stringThe outer condition gates on
props.groupId, but theparamNameternary on lines 17–20 shows that whenqueryStringis a string (an explicit param name),groupIdis not needed. As confirmed by Docusaurus docs,queryString={true}usesgroupIdas a fallback, butqueryString="param-name"is self-contained and does not requiregroupId. With the current guard, any<Tabs queryString="explicit-param">withoutgroupIdsilently skips URL reading.🛠 Proposed fix
- if (isBrowser && props.queryString && props.groupId) { + if (isBrowser && props.queryString) { const paramName = typeof props.queryString === 'string' ? props.queryString : props.groupId; + if (!paramName) return; const params = new URLSearchParams(window.location.search); const qsValue = params.get(paramName); if (qsValue) { defaultValue = qsValue; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/theme/Tabs/index.tsx` around lines 16 - 26, The URL-read guard in Tabs (docs/src/theme/Tabs/index.tsx) is too strict: change the outer if so it runs when isBrowser && props.queryString is set, not requiring props.groupId; specifically replace the current condition (isBrowser && props.queryString && props.groupId) with a check like (isBrowser && props.queryString && (typeof props.queryString === 'string' || props.groupId)) so that explicit queryString strings (e.g., queryString="param-name") are honored while keeping the existing fallback behavior where queryString===true uses groupId; keep the paramName ternary and URLSearchParams logic as-is.
30-40:rAFin hash-scroll effect may fire before the Docusaurus-triggered re-render completesDocusaurus's own queryString
useEffect(in the childTabscomponent) callssetSelectedValue(), which schedules a new React render to make the target tab's content visible. React child effects fire before parent effects, so Docusaurus's effect runs first — butsetSelectedValue()only queues the render; the DOM is not updated yet whenrequestAnimationFramein this parent effect fires.
requestAnimationFramefires before the next browser paint, which may be before React has committed the newly-selected tab's content. Consider a shortsetTimeout(e.g., 0 ms) orqueueMicrotaskas an alternative, or better yet, use auseLayoutEffect-aware approach that waits for React to finish committing the pending state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/theme/Tabs/index.tsx` around lines 30 - 40, The hash-scroll useEffect in Tabs/index.tsx currently uses requestAnimationFrame which can run before Docusaurus's child Tabs effect has committed the new selected tab (setSelectedValue), so change the scheduling to run after React has committed: replace the requestAnimationFrame callback with a macrotask (e.g., setTimeout(() => el.scrollIntoView(), 0)) or queueMicrotask depending on desired timing so the DOM for the selected tab is present when scrollIntoView runs; update the useEffect (the hash-scroll effect that decodes window.location.hash and finds el via document.getElementById) accordingly to use the new scheduler.docs/docs/guides/repository.mdx (1)
394-411: Single-option<Tabs queryString>adds URL noise without benefit.Each of these three
<Tabs groupId="deployment" queryString>blocks contains only a"Using Docker Compose"<TabItem>. WithqueryStringenabled, every visit appends?deployment=Using+Docker+Composeto the URL, but there is nothing else to select. Consider either unwrapping the content or adding a Kubernetes alternative if one is planned.♻️ Example: unwrap single-tab sections
-<Tabs groupId="deployment" queryString> -<TabItem value="Using Docker Compose" default> If you are using Docker Compose, you can override part of your existing... -</TabItem> -</Tabs>Also applies to: 450-467, 506-523
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/guides/repository.mdx` around lines 394 - 411, The Tabs blocks use <Tabs groupId="deployment" queryString> wrapping only a single <TabItem value="Using Docker Compose"> which causes unnecessary ?deployment=... query string noise; remove the queryString attribute (or unwrap the content by deleting the surrounding <Tabs> / <TabItem> and leaving the plain content) for the affected blocks, or add an additional TabItem such as a Kubernetes alternative if multiple options are intended so that <Tabs groupId="deployment" queryString> actually makes sense.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/guides/accounts-permissions.mdx`:
- Line 146: Fix three user-facing text issues in the updated sections: rename
the heading "Creating and global permissions" to "Creating global permissions",
correct the sentence "Select the action you which to use" to "Select the action
you wish to use", and rename the heading "Creating and objects permissions" to
"Creating object permissions" so the wording reads naturally; update these exact
strings in the docs/guides/accounts-permissions.mdx content (look for the
headings and the sentence text) to apply the fixes.
In `@docs/docs/guides/configuration-changes.mdx`:
- Around line 190-191: The Tabs in Step 4 use groupId="deployment" which breaks
queryString tab sync with Steps 2 and 3 that use groupId="installation-method";
update the Tabs/TabItem in Step 4 (the <Tabs> and inner <TabItem> elements) to
use groupId="installation-method" so the queryString parameter (e.g.,
?installation-method=kubernetes) selects the same tab across Steps 2, 3, and 4.
In `@docs/src/theme/Tabs/index.tsx`:
- Around line 15-26: The pre-hydration override of defaultValue (the block that
reads URLSearchParams into defaultValue using isBrowser/useIsBrowser) is dead
unless you force a remount of the inner <Tabs> by changing its key; instead,
remove that override and let Docusaurus's Tabs priority handle selection,
keeping only the hash-scroll useEffect logic; specifically delete the code that
mutates defaultValue based on window.location.search (the URLSearchParams block)
and ensure you leave the existing useIsBrowser call and the hash-scroll
useEffect intact, and do not add key={String(isBrowser)} to the <Tabs> unless
you intentionally want a remount flash.
---
Outside diff comments:
In `@docs/docs/guides/object-conversion.mdx`:
- Line 235: Remove the stray shell command "git checkout -b
atg-20251020-infp-258" from the documentation content so it is not rendered as
plain text; locate the occurrence in docs/guides/object-conversion.mdx (the
literal line containing that command) and delete it or move it into a proper
fenced code block if the checkout example is intentional, ensuring the
surrounding markdown remains valid.
In `@docs/docs/guides/resource-manager.mdx`:
- Around line 153-162: The file opens an admonition with :::success that is
never closed, causing the "### Creating an IP Prefix object" heading and the
<Tabs groupId="method" queryString> block to be included inside the callout; fix
by adding a closing ::: immediately before the "### Creating an IP Prefix
object" heading (i.e., close the :::success block) so the admonition ends before
the heading and the Tabs component.
---
Nitpick comments:
In `@docs/docs/guides/repository.mdx`:
- Around line 394-411: The Tabs blocks use <Tabs groupId="deployment"
queryString> wrapping only a single <TabItem value="Using Docker Compose"> which
causes unnecessary ?deployment=... query string noise; remove the queryString
attribute (or unwrap the content by deleting the surrounding <Tabs> / <TabItem>
and leaving the plain content) for the affected blocks, or add an additional
TabItem such as a Kubernetes alternative if multiple options are intended so
that <Tabs groupId="deployment" queryString> actually makes sense.
In `@docs/src/theme/Tabs/index.tsx`:
- Around line 16-26: The URL-read guard in Tabs (docs/src/theme/Tabs/index.tsx)
is too strict: change the outer if so it runs when isBrowser &&
props.queryString is set, not requiring props.groupId; specifically replace the
current condition (isBrowser && props.queryString && props.groupId) with a check
like (isBrowser && props.queryString && (typeof props.queryString === 'string'
|| props.groupId)) so that explicit queryString strings (e.g.,
queryString="param-name") are honored while keeping the existing fallback
behavior where queryString===true uses groupId; keep the paramName ternary and
URLSearchParams logic as-is.
- Around line 30-40: The hash-scroll useEffect in Tabs/index.tsx currently uses
requestAnimationFrame which can run before Docusaurus's child Tabs effect has
committed the new selected tab (setSelectedValue), so change the scheduling to
run after React has committed: replace the requestAnimationFrame callback with a
macrotask (e.g., setTimeout(() => el.scrollIntoView(), 0)) or queueMicrotask
depending on desired timing so the DOM for the selected tab is present when
scrollIntoView runs; update the useEffect (the hash-scroll effect that decodes
window.location.hash and finds el via document.getElementById) accordingly to
use the new scheduler.
| @@ -145,7 +145,7 @@ For a complete list of available global and object permissions, see the [Roles a | |||
|
|
|||
| ### Creating and global permissions | |||
Contributor
There was a problem hiding this comment.
Minor pre-existing text-quality issues in modified sections
Three user-facing strings in the sections updated by this PR contain errors:
- Line 146 heading:
"Creating and global permissions"— the spurious "and" is awkward; should be "Creating global permissions". - Line 155:
"Select the action you which to use"— "which" → "wish". - Line 190 heading:
"Creating and objects permissions"— should be "Creating object permissions".
✏️ Proposed fixes
-### Creating and global permissions
+### Creating global permissions- 4. Select the action you which to use.
+ 4. Select the action you wish to use.-### Creating and objects permissions
+### Creating object permissionsAlso applies to: 155-155, 190-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/docs/guides/accounts-permissions.mdx` at line 146, Fix three user-facing
text issues in the updated sections: rename the heading "Creating and global
permissions" to "Creating global permissions", correct the sentence "Select the
action you which to use" to "Select the action you wish to use", and rename the
heading "Creating and objects permissions" to "Creating object permissions" so
the wording reads naturally; update these exact strings in the
docs/guides/accounts-permissions.mdx content (look for the headings and the
sentence text) to apply the fixes.
Contributor
Author
|
@opsmill/frontend can I get a review please |
This allows sharing anchored URLs pointing to non-default tabs. Signed-off-by: Fatih Acar <fatih@opsmill.com>
fatih-acar
force-pushed
the
fac-fix-anchored-links-docs
branch
from
0efeffb to
0fc464d
Compare
However, shared links with query parameters (e.g., ?method=graphql) require visiting the page twice before the correct tab is shown.
Root cause: Docusaurus's SSG hydration forces search: '' in the server snapshot (useHistorySelector in historyUtils.js) to avoid React hydration mismatches. The useTabs hook initializes state with defaultValue (the first tab), then syncs the query string value only AFTER the initial render via useIsomorphicLayoutEffect. This means:
Pre-rendered HTML always shows the default tab
On first client render, useState initializes with defaultValue, not the query string
The query string value is applied after mount, but any hash-anchor scroll has already failed because the target content was hidden
Fix: Swizzle Tabs component with a --wrap wrapper
Create docs/src/theme/Tabs/index.tsx — a thin wrapper around the original Tabs that:
Reads the query string from window.location.search on the client
Passes the matching value as defaultValue to the original Tabs component (so useState initializes correctly on the client-side mount)
After hydration, re-scrolls to hash anchors (since the correct tab is now visible)
Why this works
Docusaurus's Tabs component uses key={String(isBrowser)} which causes a full remount after hydration. On the server/hydration pass, isBrowser=false so our wrapper passes the original defaultValue. After hydration, React remounts with isBrowser=true, our wrapper reads the query string from window.location.search, and the original Tabs receives the correct defaultValue for its useState initializer.
The --wrap swizzle pattern is Docusaurus's official extension mechanism — safe across upgrades.
Signed-off-by: Fatih Acar <fatih@opsmill.com>
fatih-acar
force-pushed
the
fac-fix-anchored-links-docs
branch
from
0fc464d to
7fceed8
Compare
bilalabbad
approved these changes
Feb 27, 2026
fatih-acar
added a commit
to opsmill/infrahub-docs
that referenced
this pull request
Feb 27, 2026
Related to opsmill/infrahub#8429 Signed-off-by: Fatih Acar <fatih@opsmill.com>
fatih-acar
added a commit
to opsmill/infrahub-docs
that referenced
this pull request
Feb 27, 2026
Related to opsmill/infrahub#8429 Signed-off-by: Fatih Acar <fatih@opsmill.com>
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
2 participants
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.
This allows sharing anchored URLs pointing to non-default tabs.
Summary by CodeRabbit