Conversation
|
@coderabbitai review again |
|
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughRestructures site docs and UI: adds quickstarts and deployment modes, a solution-design stepper with interactive architecture diagrams, new welcome page and many sidebar configs, updates Docusaurus/Webpack setup and styles, and introduces multiple new React components and CSS modules. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
docusaurus.config.ts (1)
19-20:⚠️ Potential issue | 🟠 MajorKeep broken-link failures enabled in CI.
Line 19 and Line 20 remove the one automated check most likely to catch the navigation regressions this redesign is introducing. If CI uses the same config, broken links will ship silently. Gate this on
process.env.CIinstead of relaxing it globally.Suggested change
- onBrokenLinks: "warn", - onBrokenMarkdownLinks: "warn", + onBrokenLinks: process.env.CI ? "throw" : "warn", + onBrokenMarkdownLinks: process.env.CI ? "throw" : "warn",Verify whether CI shares this config without an override. Expected result: CI builds should still fail on broken links even if local rebuilds only warn.
#!/bin/bash set -euo pipefail echo "Current broken-link policy in docusaurus.config.ts:" sed -n '17,22p' docusaurus.config.ts echo echo "Any env-specific override already present?" rg -n -C2 'onBrokenLinks|onBrokenMarkdownLinks|process\.env\.CI|CI' docusaurus.config.ts || true echo echo "Build commands in package scripts:" if [ -f package.json ]; then rg -n -C2 'docusaurus build|npm run build|pnpm build|yarn build' package.json || true fi echo echo "Build commands in GitHub workflows:" if [ -d .github ]; then rg -n -C2 'docusaurus build|npm run build|pnpm build|yarn build' .github || true fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docusaurus.config.ts` around lines 19 - 20, The config currently sets onBrokenLinks and onBrokenMarkdownLinks to "warn" globally; change this so CI still fails: update the values for onBrokenLinks and onBrokenMarkdownLinks in docusaurus.config.ts to use a conditional based on process.env.CI (e.g., use "error" when process.env.CI is truthy, otherwise "warn") so local builds warn but CI builds fail; locate the properties named onBrokenLinks and onBrokenMarkdownLinks in the exported config object and replace the static "warn" strings with the CI-gated expression.
🟠 Major comments (19)
.coderabbit.yaml-4-5 (1)
4-5:⚠️ Potential issue | 🟠 MajorGlobal MDX exclusion removes review coverage from the docs surface.
Lines 4–5 exclude every
*.mdxfile, which suppresses review on the main docs content in a docs-focused repo/PR. This can hide navigation and link regressions.Suggested adjustment
reviews: review_status: false - # Temporarily exclude MDX files from review processing. + # Keep MDX in scope; only exclude generated/vendor artifacts if needed. path_filters: - - "!**/*.mdx" + - "!**/node_modules/**" + - "!**/build/**" + - "!**/.docusaurus/**"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.coderabbit.yaml around lines 4 - 5, The global path_filters entry currently negates all MDX files via the pattern "!**/*.mdx", which removes review coverage for docs; update the .coderabbit.yaml by removing or narrowing that exclusion (e.g., delete the "!**/*.mdx" pattern or replace it with more specific exclusions such as "!public/**.mdx" or an allow-list for your docs directory) so that documentation MDX files are included in reviews; look for the path_filters key and the pattern "!**/*.mdx" to make the change.src/pages/_assets/product-selector-graph.module.css-25-25 (1)
25-25:⚠️ Potential issue | 🟠 MajorConfigure stylelint to allow CSS Modules
:global()pseudo-class.The file uses
:global()selectors extensively (lines 25, 34, 41, 46–252) to style global Mermaid classes within the CSS Module. The stylelint configuration does not currently ignore theglobalpseudo-class, causingselector-pseudo-class-no-unknownto treat it as invalid and block CI.Add the following to
.stylelintrc.json:"selector-pseudo-class-no-unknown": [ true, { "ignorePseudoClasses": ["global"] } ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/_assets/product-selector-graph.module.css` at line 25, The Stylelint rule is flagging the CSS Modules :global() pseudo-class used in selectors like ".graphContainer :global(.mermaid) :global(.cluster)"; update the Stylelint config (in .stylelintrc.json) to allow the "global" pseudo-class by adding "global" to the selector-pseudo-class-no-unknown ignorePseudoClasses array so stylelint no longer treats :global as unknown.src/pages/_assets/product-selector-graph.module.css-142-145 (1)
142-145:⚠️ Potential issue | 🟠 MajorRemove redundant
backgroundshorthand declarations that conflict withbackground-colorlonghand.The
declaration-block-no-shorthand-property-overridesrule (enabled viastylelint-config-standard-scss) flags when a shorthand property overrides a longhand property in the same declaration block. The redundantbackground: transparent !important;should be removed from all four affected locations, keeping onlybackground-colorfor clarity.🩹 Proposed fix
.graphContainer :global(.mermaid) :global(.edgeLabel) { background-color: transparent !important; - background: transparent !important; color: var(--ifm-color-emphasis-800) !important; font-size: 11px !important; font-weight: 600 !important; @@ .graphContainer :global(.mermaid) :global(.edgeLabel) :global(foreignObject) :global(div) { background-color: transparent !important; - background: transparent !important; } @@ :global([data-theme="dark"]) .graphContainer :global(.mermaid) :global(.edgeLabel) { background-color: transparent !important; - background: transparent !important; color: var(--ifm-color-emphasis-100) !important; border: none !important; } @@ :global([data-theme="dark"]) .graphContainer :global(.mermaid) :global(.edgeLabel) :global(foreignObject) :global(div) { background-color: transparent !important; - background: transparent !important; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/_assets/product-selector-graph.module.css` around lines 142 - 145, Remove the redundant background shorthand that overrides the longhand background-color in the CSS rule for .graphContainer :global(.mermaid) :global(.edgeLabel) (and the three other similar selectors flagged in this file); keep only background-color: transparent !important; and color as-is so the declaration-block-no-shorthand-property-overrides lint rule is satisfied and the longhand value is not shadowed by a subsequent background shorthand.docs/products/product-arch-28-57 (1)
28-57:⚠️ Potential issue | 🟠 MajorMermaid diagram won’t render without a fenced block.
Line 28 starts Mermaid syntax directly, but it must be wrapped in a ```mermaid code fence to render.
Proposed fix
-flowchart TD +```mermaid +flowchart TD Start([What are you building?]) --> Q1{Need to manage<br>user identities?} Q1 -->|Yes| Kratos[✓ Kratos] @@ Q5 -->|Yes| Oathkeeper[✓ Add Oathkeeper] Q5 -->|No| Done([Your stack is ready]) Oathkeeper --> Done +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/products/product-arch` around lines 28 - 57, The Mermaid diagram starting with "flowchart TD" (nodes like Start, Q1, Kratos, Polis, Hydra, Keto, Oathkeeper, Done) must be wrapped in a fenced Mermaid code block; update the doc so the entire diagram is enclosed between a starting ```mermaid line and a closing ``` line (no other changes to node names or arrows), ensuring the exact sequence from "flowchart TD" through "Oathkeeper --> Done" sits inside that fence so the renderer can parse it.docs/oss/index.md-1-4 (1)
1-4:⚠️ Potential issue | 🟠 MajorAvoid shipping an empty OSS landing page.
With
hide_title: trueon Line 3 and no body content, this route renders as a blank page. Add at least a short intro (or removehide_title) so/docs/ossis navigable.Proposed fix
--- title: Ory Open Source hide_title: true --- + +Welcome to Ory Open Source docs. Start with deployment, configuration, and product guides tailored for OSS users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/oss/index.md` around lines 1 - 4, The OSS docs page currently contains only frontmatter with hide_title: true which results in a blank route; either add a short introductory paragraph below the YAML frontmatter in docs/oss/index.md (e.g., a one- or two-sentence intro about the Ory open source offering) or remove the hide_title: true key so the title is shown—update the file so /docs/oss renders non-empty content.src/components/Shared/kratos/e2e-integration-tests.tsx-18-41 (1)
18-41:⚠️ Potential issue | 🟠 MajorDon't render a self-hosted setup example for the
networkproduct.Lines 18–41 set
commandPrefixto"ory"for Ory Network and"kratos"for self-hosted deployments. However, the surrounding text (lines 35–36) describes booting Ory Kratos locally withDSN=memoryand a config file—a self-hosted setup that is incorrect for Ory Network, which is a managed service. Line 40 will produceory serve -c ./path/to/config/kratos.yml --devfor thenetworkbranch, which is not a valid command pattern.Additionally, line 76 contains a hardcoded
hydra servecommand (should bekratos serveor use thecommandPrefixvariable like line 40 does).Either restrict this entire section to
oel | ossproducts, or provide separate Network-specific instructions that don't assume self-hosted operation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Shared/kratos/e2e-integration-tests.tsx` around lines 18 - 41, The text and example command incorrectly present a self-hosted Kratos startup for the "network" product by using commandPrefix (set to "ory" for network) and a hardcoded "hydra serve" elsewhere; update the component so this section only renders for self-hosted products (check product values "oel" | "oss") or split into two branches: for self-hosted use the existing DSN=memory ${commandPrefix} serve example (with commandPrefix set to "kratos") and for Network render an explicit note and Network-specific instructions (no local serve command). Also replace the hardcoded "hydra serve" occurrence to use the commandPrefix variable or the correct "kratos serve" in the self-hosted branch and ensure the UI text never suggests running a local Kratos server when product === "network".src/components/QuickStarts/hooks/useClickOutside.ts-3-6 (1)
3-6:⚠️ Potential issue | 🟠 MajorAccept nullable refs from callers.
Line 4 is typed as
RefObject<HTMLElement>, but actual callers inDeploymentModeSelector.tsxandLanguageFilter.tsxuseuseRef<HTMLDivElement | null>(null), which yieldsRefObject<HTMLDivElement | null>. This causes a type mismatch. The hook implementation already safely handles null refs via the guard clauseif (ref.current && ...)on line 14, but the signature should reflect this. Make the hook generic to accept nullable refs.🛠️ Proposed fix
-export function useClickOutside( - ref: RefObject<HTMLElement>, +export function useClickOutside<T extends HTMLElement>( + ref: RefObject<T | null>, isOpen: boolean, onClose: () => void, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/hooks/useClickOutside.ts` around lines 3 - 6, The hook signature for useClickOutside currently requires RefObject<HTMLElement> but callers pass nullable refs; update the signature to accept nullable refs by making the hook generic (e.g., useClickOutside<T extends HTMLElement | null>) and change the ref parameter type to RefObject<T> (or RefObject<T | null>) so callers using useRef<HTMLDivElement | null>(null) match; keep the existing null-guard logic inside the function and ensure the generic is used in the function definition and any exported typing to preserve callers' inferred element type.src/components/Shared/kratos/e2e-integration-tests.tsx-66-77 (1)
66-77:⚠️ Potential issue | 🟠 MajorFix the command snippet and update prose for environment variable value.
The code block has multiple issues:
- Uses
hydra serveinstead ofkratos serve- Stray
)at end of command- Indentation breaks shell syntax
- Prose contradicts code by stating
DEV_DISABLE_API_FLOW_ENFORCEMENT=1, but per Ory documentation, boolean environment variables require=true(or=false), not=1Update both the prose and code block:
🛠️ Proposed fix
<p> If you want to test React Native (rendered as a web application) in Cypress, you need to disable security features preventing browser from executing self-service API flows. To do this, set the environment - variable <code>DEV_DISABLE_API_FLOW_ENFORCEMENT=1</code>: + variable <code>DEV_DISABLE_API_FLOW_ENFORCEMENT=true</code>: </p> <CodeBlock language="sh"> {`DSN=memory \\ - DEV_DISABLE_API_FLOW_ENFORCEMENT=true - hydra serve -c ./path/to/config/kratos.yml --dev > "kratos.e2e.log" 2>&1 &)`} +DEV_DISABLE_API_FLOW_ENFORCEMENT=true \\ +kratos serve -c ./path/to/config/kratos.yml --dev > "kratos.e2e.log" 2>&1 &`} </CodeBlock>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Shared/kratos/e2e-integration-tests.tsx` around lines 66 - 77, Update the prose to say DEV_DISABLE_API_FLOW_ENFORCEMENT=true (not =1) and fix the shell snippet inside the CodeBlock: replace the incorrect "hydra serve" with "kratos serve", remove the stray trailing ")" and correct line breaks/indentation so environment variables (DSN=memory and DEV_DISABLE_API_FLOW_ENFORCEMENT=true) are exported/set before invoking kratos, and ensure the kratos serve command uses the correct flags and backgrounding/redirect syntax (e.g., proper use of & and redirections) so the snippet is a valid shell command; locate and edit the CodeBlock component and surrounding explanatory paragraph in kratos/e2e-integration-tests.tsx.src/components/OryNetworkCta/ory-network-cta.tsx-17-21 (1)
17-21:⚠️ Potential issue | 🟠 MajorDon't call
useBaseUrl()conditionally.Lines 17-21 invoke
useBaseUrl()only when the imported asset is not a string. SinceuseBaseUrlis a React hook, this violates the Rules of Hooks—hooks must always be called unconditionally at the top level of a component. Move theuseBaseUrl()call outside the conditional and reuse the result.🛠️ Proposed fix
export const OryNetworkCta = () => { const { title, description, ctaLabel, href } = CTA_CONFIG + const fallbackImageSrc = useBaseUrl("/img/network-cta/network.png") const imageSrc = typeof networkImg === "string" ? networkImg - : (networkImg as { default?: string }).default ?? - useBaseUrl("/img/network-cta/network.png") + : (networkImg as { default?: string }).default ?? fallbackImageSrc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryNetworkCta/ory-network-cta.tsx` around lines 17 - 21, The component currently calls useBaseUrl() conditionally inside the imageSrc expression which breaks the Rules of Hooks; call useBaseUrl once at the top of the component (e.g., const baseNetworkImg = useBaseUrl("/img/network-cta/network.png")) and then compute imageSrc using networkImg and baseNetworkImg (use typeof networkImg === "string" ? networkImg : (networkImg as { default?: string }).default ?? baseNetworkImg) so useBaseUrl is invoked unconditionally and imageSrc logic is unchanged.src/components/Shared/self-hosted/deployment.tsx-172-179 (1)
172-179:⚠️ Potential issue | 🟠 MajorThis shared section hard-codes OEL-only navigation into every deployment page.
DeploymentDatabaseis shared acrossnetwork,oel, andoss, but this block always renders OEL-specific copy../oel/high-performance-poolingresolves relative to the current page, and the#high-performance-poolingjump on Line 200 has no matchingidon Line 172. Gate this section byproduct(or compute a deployment-aware destination) and add an explicit heading id.Also applies to: 195-204
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Shared/self-hosted/deployment.tsx` around lines 172 - 179, The shared DeploymentDatabase component currently renders OEL-specific content unconditionally and uses a relative ./oel/high-performance-pooling link plus a mismatched anchor; update DeploymentDatabase to only render the "High-performance pooling" block when product === "oel" (or compute a deployment-aware destination based on the current page context), change the link to an explicit deployment-aware path rather than a relative "./oel/..." URL, and add a matching id attribute (e.g., id="high-performance-pooling") to the <h5> heading so the `#high-performance-pooling` anchor works; ensure the same gating and id fix is applied to the other repeated block around lines 195-204.sidebars-quickstarts.ts-52-58 (1)
52-58:⚠️ Potential issue | 🟠 MajorDon't send the Network sidebar into OEL docs.
This
Ory NetworkPolis section mixes a Network landing page with an OEL quickstart. Following that second item switches deployment model from inside the Network sidebar and undermines the split-navigation structure this PR introduces. Keep this section network-only, or drop the second entry until a Network-specific quickstart exists.🧭 Suggested fix
items: [ "network/polis/quickstarts/index", - "oel/polis/quickstarts/quickstart", ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sidebars-quickstarts.ts` around lines 52 - 58, The "Enterprise SSO AuthZ" Network sidebar currently mixes Network and OEL content by including "oel/polis/quickstarts/quickstart"; remove that entry from the items array (leave only "network/polis/quickstarts/index") so the Network sidebar remains network-only, or alternatively move the "oel/polis/quickstarts/quickstart" entry into the appropriate OEL sidebar instead; update the items list in sidebars-quickstarts.ts for the "Enterprise SSO AuthZ" category accordingly.src/components/welcomePage/ContentSection.tsx-10-15 (1)
10-15:⚠️ Potential issue | 🟠 MajorUse a stable Console signup URL here.
The
flow=UUID is transient state. Baking it into a homepage CTA means this card will eventually send users to an expired registration flow instead of starting signup. Point this at a stable Console entry URL and let Console mint a fresh flow.🔗 Suggested fix
- to: "https://console.ory.sh/registration?flow=e27eca2b-ca8a-4722-8a4e-9c52a20aa0d4", + to: "https://console.ory.sh/registration",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/ContentSection.tsx` around lines 10 - 15, The "Ory Console" card uses a transient registration flow URL (the `to` property containing `flow=<UUID>`), which will expire; update the `to` value in the object with label "Ory Console" inside ContentSection (the card definition) to a stable Console entry URL (e.g., remove the `flow=` query and point to a persistent entry like https://console.ory.sh/registration or https://console.ory.sh/) so Console can mint a fresh flow when users click the CTA.sidebars.ts-7-10 (1)
7-10:⚠️ Potential issue | 🟠 MajorAvoid importing Docusaurus internal
srctypes.Line 7 imports from
@docusaurus/plugin-content-docs/src/sidebars/types, which is an internal module and not supported for public use. This can break on dependency updates. Use the publicSidebarsConfigAPI instead.🔧 Proposed refactor (public API only)
import type { SidebarsConfig } from "@docusaurus/plugin-content-docs" - -import { SidebarItemConfig } from "@docusaurus/plugin-content-docs/src/sidebars/types" - -type SidebarItemsConfig = SidebarItemConfig[] +type SidebarItemsConfig = SidebarsConfig[string]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sidebars.ts` around lines 7 - 10, Replace the internal import of SidebarItemConfig with the public SidebarsConfig API: remove the import from "@docusaurus/plugin-content-docs/src/sidebars/types" and instead import SidebarsConfig from the public package (e.g., "@docusaurus/plugin-content-docs"), then update the type alias SidebarItemsConfig to use the public SidebarsConfig type (or derive the specific item type from SidebarsConfig if you need a narrower shape) so the file no longer depends on internal src types.sidebars-oss.ts-55-60 (1)
55-60:⚠️ Potential issue | 🟠 MajorKeep the OSS sidebar inside OSS/shared docs.
These entries live under “Ory Open Source” but send users to
oel/...andself-hosted/oel/...pages. That makes the new deployment split feel broken as soon as someone clicks into a product from the OSS nav.Also applies to: 134-142, 205-211, 251-255, 427-430
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sidebars-oss.ts` around lines 55 - 60, The OSS sidebar currently contains entries that reference OEL pages ("oel/kratos/intro", "kratos/install", "self-hosted/oel/kratos/upgrade", "self-hosted/oel/kratos/changelog", "oel/kratos/quickstart") which breaks the OSS/shared split; move or replace these entries so the OSS sidebar only links to OSS/shared docs — either remove them from the OSS sidebar array and add equivalent links to the OEL/self-hosted sidebar, or update the ids to the correct OSS/shared ids, and apply the same fix for the other occurrences noted (the other blocks that reference "oel/..." or "self-hosted/oel/..."). Ensure the references in the sidebar data structure (the items with type: "ref" and string IDs) point to the proper namespace (OSS/shared) or are relocated to the OEL sidebar.src/navbar.ts-30-31 (1)
30-31:⚠️ Potential issue | 🟠 MajorMake the Products dropdown target absolute.
Unlike the other top-level dropdowns, this
tovalue is relative. From a nested docs page it will resolve against the current URL instead of/docs/products/products-overview.Suggested fix
- to: "products/products-overview", + to: "/docs/products/products-overview",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/navbar.ts` around lines 30 - 31, The Products dropdown entry currently uses a relative `to` value ("products/products-overview") which can resolve incorrectly from nested docs pages; update the dropdown item (the object with type: "dropdown" and its `to` property in src/navbar.ts) to use the absolute docs path (e.g. "/docs/products/products-overview") consistent with the other top-level dropdowns so it always targets the correct top-level products overview page.src/components/QuickStarts/quickstart-filter.tsx-107-124 (1)
107-124:⚠️ Potential issue | 🟠 MajorClamp
languageto the current category.The effect mirrors any
?language=value into state without checkingavailableLanguages. After a category/deployment change, a stale language can blank outfilteredByLanguage, and if the new category only has one language the picker disappears so the user can't recover from the empty state.Also applies to: 137-141
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/quickstart-filter.tsx` around lines 107 - 124, The effect handling URL params sets activeLanguage directly from getSearchParams without validating it against the selected category's availableLanguages, which can leave activeLanguage stale/invalid after category changes; update the useEffect (the hook that reads location.search) to clamp the urlLanguage to the current category by checking visibleCategories / the chosen category's availableLanguages (or a derived availableLanguages array) before calling setActiveLanguage, and if the urlLanguage is not in that list fall back to a valid language (e.g., the category default or "all"); similarly ensure the same validation logic is applied in the other effect referenced (lines 137-141) and keep languageFilterRef.current?.close(), setActiveCategoryId, and setActiveLanguage calls intact but only after computing the validated language.src/components/welcomePage/SelectFrameworkBlock.tsx-30-37 (1)
30-37:⚠️ Potential issue | 🟠 MajorUse a real default selection here.
Falling back to
frameworks[1]makes the snippet/guide render from the second option while no tab is active, and it crashes when only one framework is passed. Default toframeworks[0]and guard the empty-array case.Suggested fix
- const selected = - frameworks.find((f) => f.value === selectedFramework) ?? frameworks[1] + const selected = + frameworks.find((f) => f.value === selectedFramework) ?? frameworks[0] + if (!selected) { + return null + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/SelectFrameworkBlock.tsx` around lines 30 - 37, The current fallback uses frameworks[1] which can render the wrong item or crash when frameworks has length < 2; change the selection fallback to use frameworks[0] and guard an empty array: in SelectFrameworkBlock replace the selected assignment that references frameworks[1] with one that falls back to frameworks[0] and then to a safe default object (e.g., { value: "", snippet: "" }) when frameworks is empty, so selected, snippetPrefix, and snippetRest never access undefined; keep the existing snippetPrefixes/snippetPrefix/snippetRest logic but ensure they read from the safe selected.snippet.src/components/OryArchitectureDiagram.tsx-36-47 (1)
36-47:⚠️ Potential issue | 🟠 MajorAdd
identityAnswertoFitViewOnChangedependency and lower theminZoomclamp.
identityAnswercan add/remove the external IdP and switch the graph between compact and multi-row layouts, butFitViewOnChangeonly watchesselectedProducts. The hardminZoom: 1clamp also prevents the wider layout from fitting on narrow viewports.Include
identityAnswerin the dependency array and pass it toFitViewOnChangeat line 231. Review and likely lower theminZoomvalue across allfitViewcalls.Also applies to: 72–74, 226–227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryArchitectureDiagram.tsx` around lines 36 - 47, FitViewOnChange only depends on selectedProducts but must also depend on identityAnswer because adding/removing the external IdP changes layout; update the dependency array in FitViewOnChange to include identityAnswer and update any call sites that render/use <FitViewOnChange ...> (pass the identityAnswer prop where FitViewOnChange is instantiated), and relax the hard minZoom clamp (lower from 1 to a smaller value such as 0.5) in all fitView({ padding..., duration..., minZoom }) invocations so wider layouts can fit on narrow viewports; locate references by the FitViewOnChange function name, the identityAnswer variable, and fitView from useReactFlow.src/components/OryArchitectureDiagram.tsx-191-198 (1)
191-198:⚠️ Potential issue | 🟠 MajorGuard the fullscreen API before chaining
.then().
requestFullscreen?.()/exitFullscreen?.()can returnundefined, and both promises can reject. On unsupported or denied browsers this turns the fullscreen button into a runtime error path. The optional chaining?.()evaluates toundefinedif the method doesn't exist, and calling.then()onundefinedthrows aTypeError.Suggested fix
- const toggleFullscreen = useCallback(() => { + const toggleFullscreen = useCallback(async () => { const el = containerRef.current if (!el) return - if (!document.fullscreenElement) { - el.requestFullscreen?.().then(() => setIsFullscreen(true)) - } else { - document.exitFullscreen?.().then(() => setIsFullscreen(false)) + try { + if (!document.fullscreenElement) { + if (el.requestFullscreen) { + await el.requestFullscreen() + } + } else if (document.exitFullscreen) { + await document.exitFullscreen() + } + } catch { + // Ignore rejected / unsupported fullscreen requests. } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryArchitectureDiagram.tsx` around lines 191 - 198, The toggleFullscreen function uses optional chaining to call el.requestFullscreen?.() and document.exitFullscreen?.() but immediately chains .then(), which will throw if the call returns undefined or rejects; update toggleFullscreen to first check that the respective methods exist (el.requestFullscreen and document.exitFullscreen), call them only if present, capture their return value into a variable (promise), and then safely handle it by using Promise.resolve(promise).then(() => setIsFullscreen(true/false)).catch(() => {/* handle or log error and avoid changing state on failure */}); ensure you reference and update the toggleFullscreen function, containerRef, requestFullscreen, exitFullscreen, and setIsFullscreen usages.
🟡 Minor comments (9)
src/components/welcomePage/GuideLinkBox.tsx-12-22 (1)
12-22:⚠️ Potential issue | 🟡 MinorMake the whole card the link.
Lines 12-22 style this as a CTA box, but only the inline text/icon is clickable. The padded body becomes dead space, which is easy to miss on the welcome page. Wrapping the card itself in
Linkwill match the visual affordance.💡 Proposed fix
export function GuideLinkBox({ to, label }: GuideLinkBoxProps) { return ( - <div className="mt-ory-4 flex flex-col gap-ory-3 p-ory-6 rounded-ory border border-ory-border-info-tertiary bg-ory-info-tertiary"> - <Link - to={to} - className="ory-body-sm font-medium text-ory-info-on-tertiary no-underline inline-flex items-center gap-ory-2" - > + <Link + to={to} + className="mt-ory-4 flex flex-col gap-ory-3 rounded-ory border border-ory-border-info-tertiary bg-ory-info-tertiary p-ory-6 no-underline" + > + <span className="ory-body-sm inline-flex items-center gap-ory-2 font-medium text-ory-info-on-tertiary"> <IconBulb - className="w-5 h-5 shrink-0 text-ory-info-primary" + className="h-5 w-5 shrink-0 text-ory-info-primary" aria-hidden /> Follow the quickstart guide for {label} - </Link> - </div> + </span> + </Link> ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/GuideLinkBox.tsx` around lines 12 - 22, The CTA card currently only makes the inline text/icon clickable; wrap the entire card so the whole padded area is a link by replacing the outer <div className="..."> with the <Link to={to} className="..."> (or alternatively wrap the div with a single Link) and remove the inner Link to avoid nested anchors; keep the same class names, IconBulb, and the "Follow the quickstart guide for {label}" text so the full card (GuideLinkBox) is clickable and styling/ARIA are preserved.src/components/OryHeroDemo.jsx-132-135 (1)
132-135:⚠️ Potential issue | 🟡 MinorExpose the terminal stream as a live log.
The demo appends content in place, but this container is just a generic
div. Screen-reader users won't be told that output changed or that the final CTA appeared. Mark it as a log/live region and surface running state.♿ Suggested fix
<div ref={terminalRef} + role="log" + aria-live="polite" + aria-relevant="additions text" + aria-busy={isRunning} className="p-4 font-mono text-[13px] text-ory-text-primary bg-ory-bg-primary leading-[1.6] overflow-y-auto flex-1 min-h-0" >Also applies to: 189-199, 204-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/OryHeroDemo.jsx` around lines 132 - 135, The terminal output container (the div referenced by terminalRef) needs ARIA live-region semantics so screen-readers announce updates; add role="log" and aria-live="polite" (or "assertive" if announcements must interrupt) plus aria-atomic="true" or "false" as appropriate, and bind aria-busy to the component's running state (e.g., isStreaming / isRunning) so updates reflect live activity; apply the same changes to the other output containers mentioned (the divs around lines 189-199 and 204-209) and ensure the running state variable used in aria-busy is updated when the stream starts/stops.src/components/welcomePage/OverviewCard.tsx-49-57 (1)
49-57:⚠️ Potential issue | 🟡 MinorGive each inline CTA a unique accessible name.
Every welcome-page card currently renders the same
"Learn more"link text here, so assistive tech gets a list of indistinguishable links with no card context. Includetitlein the link’s accessible name, or make the heading part of the link.♿ Suggested fix
- <Link to={to} className="ory-guide-card__link mt-auto"> + <Link + to={to} + aria-label={`${linkLabel} about ${title}`} + className="ory-guide-card__link mt-auto" + > {linkLabel} <span aria-hidden>→</span> </Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/OverviewCard.tsx` around lines 49 - 57, The link text "Learn more" is ambiguous to screen reader users; update the OverviewCard component so each CTA has a unique accessible name by adding a descriptive title attribute to the Link (e.g., title={`${title} - ${linkLabel}`}) or alternatively by making the card heading part of the link so the heading (title) and linkLabel are read together; modify the Link element (the JSX that uses variables linkLabel, to, wrapperClass, innerClass, content, and title) to include the chosen accessible-name change and ensure the aria-hidden decorative arrow remains aria-hidden.src/components/welcomePage/SelectFrameworkSection.tsx-17-20 (1)
17-20:⚠️ Potential issue | 🟡 MinorFix TypeScript capitalization in user-facing copy.
Line 17 and Line 20 use
Typescript; standard casing isTypeScript. This affects polish and search consistency.💡 Proposed fix
- label: "Typescript", + label: "TypeScript", ... - guideTitle: "Follow the quickstart guide for Typescript", + guideTitle: "Follow the quickstart guide for TypeScript",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/SelectFrameworkSection.tsx` around lines 17 - 20, The displayed user-facing strings use "Typescript" with incorrect capitalization; update the values for the "label" and "guideTitle" properties in SelectFrameworkSection (the entries named label and guideTitle) to "TypeScript" (capital S) so both the option label and the quickstart guide text use the correct casing and remain consistent for search and polish.src/components/QuickStarts/constants.ts-109-113 (1)
109-113:⚠️ Potential issue | 🟡 MinorFix the Polis blurb typo.
“SCIM provising” should be “SCIM provisioning”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/constants.ts` around lines 109 - 113, The QuickStarts entry for label "Introduction to Ory Polis" contains a typo in the description property; change the string value of description from "Enterprise SSO for SAML and OIDC identity providers and SCIM provising." to "Enterprise SSO for SAML and OIDC identity providers and SCIM provisioning." so the description property in the QuickStarts constants (the object with label "Introduction to Ory Polis") uses the correct spelling.src/components/QuickStarts/CategoryFilter.tsx-17-31 (1)
17-31:⚠️ Potential issue | 🟡 MinorExpose the active category state.
The selected chip is only communicated by styling right now. Add
aria-pressedto the buttons so assistive tech can announce which category is currently applied.Suggested change
- <div className="flex flex-wrap items-center gap-2 mb-5"> + <div + className="flex flex-wrap items-center gap-2 mb-5" + role="group" + aria-label="Quickstart categories" + > @@ <button key={cat.id} type="button" + aria-pressed={activeCategory.id === cat.id} className={clsx(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/CategoryFilter.tsx` around lines 17 - 31, The category buttons only communicate selection visually; add an accessible state by setting the button's aria-pressed attribute inside the categories.map render so assistive tech can announce the active chip. Update the button in CategoryFilter.tsx (the mapped button using key={cat.id}, activeCategory and onCategoryChange) to include aria-pressed={activeCategory.id === cat.id} (or the equivalent boolean expression) so the selected category is exposed to assistive technologies.src/components/welcomePage/SolutionDesignStepper.tsx-159-170 (1)
159-170:⚠️ Potential issue | 🟡 MinorUse radio semantics for the answer choices.
The current selection is only visible by border color. Because this is a single-choice question, expose it as a radio group (
role="radiogroup"+role="radio"/aria-checked) or switch to actual radio inputs so screen readers can announce the selected answer.Suggested change
- <div className="flex flex-col gap-ory-3"> + <div + className="flex flex-col gap-ory-3" + role="radiogroup" + aria-label={currentQuestion.question} + > {currentQuestion.options.map((option) => { const selected = currentAnswer === option.value return ( <button key={option.value} type="button" + role="radio" + aria-checked={selected} onClick={() => handleAnswer(currentQuestion.id, option.value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/welcomePage/SolutionDesignStepper.tsx` around lines 159 - 170, The option buttons in SolutionDesignStepper render single-choice answers but lack radio semantics; update the markup around currentQuestion.options (in the SolutionDesignStepper component) to expose a proper radio group by either replacing the buttons with actual <input type="radio"> elements (wired to handleAnswer and currentAnswer) or by adding role="radiogroup" to the container and role="radio", aria-checked, and keyboard focus/keydown handling (Enter/Space) to each option element; ensure keys remain option.value and that selection styling still reflects currentAnswer so screen readers can announce the selected option.src/components/QuickStarts/LanguageFilter.tsx-37-59 (1)
37-59:⚠️ Potential issue | 🟡 MinorExpose the popup state on the trigger.
The toggle never tells assistive tech whether the language menu is open. Add
aria-expanded,aria-haspopup, and anaria-controlsrelationship to the popup container so this reads as a real disclosure instead of a generic button.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/QuickStarts/LanguageFilter.tsx` around lines 37 - 59, The button toggle in LanguageFilter.tsx should expose the popup state to assistive tech: update the toggle button (the element using onClick={() => setMenuOpen(...)} and the activeLanguage display) to include aria-expanded={menuOpen}, aria-haspopup="true", and aria-controls pointing to the popup container's id; add a stable id to the popup div (the element rendered when menuOpen is true) and ensure the popup container has that id and an appropriate role (e.g., role="menu") so the aria-controls relationship is valid.src/css/theme.css-1238-1245 (1)
1238-1245:⚠️ Potential issue | 🟡 MinorThis selector leaks the quickstart reset outside the tabs.
.tabs-container[data-group-id="language-quickstarts"] ~ * [class*="theme-code-block"]matches any descendant code block in any later sibling, so unrelated snippets further down the page inherit the quickstart styling too. Scope the reset to the tab container/panel instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/css/theme.css` around lines 1238 - 1245, The CSS selector `.tabs-container[data-group-id="language-quickstarts"] ~ * [class*="theme-code-block"]` leaks styles to later sibling content; restrict the reset to only code inside the quickstarts tab/panel by replacing the loose sibling descendant selector with one that targets the tab container/panel scope (e.g. target descendants of the `.tabs-container[data-group-id="language-quickstarts"]` panel element such as `.tabs-container[data-group-id="language-quickstarts"] .tab-panel [class*="theme-code-block"]` and likewise for the `pre`/`code` rules), so only code blocks inside the quickstarts tab inherit the quickstart styling.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/components/SidebarDeploymentModel/index.tsx (2)
77-90: Scope document listeners to open state.The click/focus-outside listeners are registered even when the menu is closed. Gate the effect by
openso listeners only exist while the dropdown is visible.♻️ Proposed refinement
- useEffect(() => { - if (!sidebar?.name || !QUICKSTARTS_SIDEBAR_NAMES.has(sidebar.name)) return + useEffect(() => { + if ( + !open || + !sidebar?.name || + !QUICKSTARTS_SIDEBAR_NAMES.has(sidebar.name) + ) { + return + } const handleClickOutside = (e: MouseEvent | TouchEvent | FocusEvent) => { if (!ref.current?.contains(e.target as Node)) setOpen(false) } document.addEventListener("mousedown", handleClickOutside) document.addEventListener("touchstart", handleClickOutside) document.addEventListener("focusin", handleClickOutside) return () => { document.removeEventListener("mousedown", handleClickOutside) document.removeEventListener("touchstart", handleClickOutside) document.removeEventListener("focusin", handleClickOutside) } - }, [sidebar?.name]) + }, [open, sidebar?.name])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SidebarDeploymentModel/index.tsx` around lines 77 - 90, The effect attaching document listeners in useEffect runs regardless of the dropdown state; restrict it to only when the dropdown is open to avoid unnecessary listeners and potential leaks. Update the useEffect condition to include the open boolean (e.g. useEffect(..., [sidebar?.name, open])) and early-return when !open or when sidebar?.name is not in QUICKSTARTS_SIDEBAR_NAMES; keep handleClickOutside using ref.current?.contains(...) and setOpen(false), and ensure you still clean up the same event listeners in the returned teardown.
108-113: Avoid custom Enter/Space toggle on a native button.A native
<button>already supports keyboard activation. The custom handler can be redundant and may cause inconsistent toggling behavior. Prefer relying ononClick.♿ Suggested simplification
- onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault() - setOpen((v) => !v) - } - }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SidebarDeploymentModel/index.tsx` around lines 108 - 113, The custom onKeyDown handler on the button in SidebarDeploymentModel toggling setOpen for Enter/Space is redundant and can conflict with native button behavior; remove the onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault(); setOpen(v => !v) } }} handler and rely on the existing onClick handler to toggle setOpen, and while editing confirm the element is an actual <button> (or add type="button") so keyboard activation and form behavior remain correct.docusaurus.config.ts (1)
125-125: Use a relative path for the footer logo to maintain compatibility with baseUrl changes.Line 125 hardcodes
/docs/in the asset path. Docusaurus v3 automatically prependsbaseUrlto relative static paths in theme config, so usingimg/logos/logo-ory-white-2022-11-04.svgmakes this configuration resilient ifbaseUrlchanges. This matches the pattern already used for the favicon in the same config.Proposed diff
- src: "/docs/img/logos/logo-ory-white-2022-11-04.svg", + src: "img/logos/logo-ory-white-2022-11-04.svg",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docusaurus.config.ts` at line 125, The footer logo src currently uses an absolute path "/docs/img/logos/logo-ory-white-2022-11-04.svg" which breaks when baseUrl changes; update the footer logo configuration in docusaurus.config.ts (the footer logo src property) to use a relative static path "img/logos/logo-ory-white-2022-11-04.svg" so Docusaurus v3 will prepend baseUrl correctly, matching how the favicon is configured.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docusaurus.config.ts`:
- Line 125: The footer logo src currently uses an absolute path
"/docs/img/logos/logo-ory-white-2022-11-04.svg" which breaks when baseUrl
changes; update the footer logo configuration in docusaurus.config.ts (the
footer logo src property) to use a relative static path
"img/logos/logo-ory-white-2022-11-04.svg" so Docusaurus v3 will prepend baseUrl
correctly, matching how the favicon is configured.
In `@src/components/SidebarDeploymentModel/index.tsx`:
- Around line 77-90: The effect attaching document listeners in useEffect runs
regardless of the dropdown state; restrict it to only when the dropdown is open
to avoid unnecessary listeners and potential leaks. Update the useEffect
condition to include the open boolean (e.g. useEffect(..., [sidebar?.name,
open])) and early-return when !open or when sidebar?.name is not in
QUICKSTARTS_SIDEBAR_NAMES; keep handleClickOutside using
ref.current?.contains(...) and setOpen(false), and ensure you still clean up the
same event listeners in the returned teardown.
- Around line 108-113: The custom onKeyDown handler on the button in
SidebarDeploymentModel toggling setOpen for Enter/Space is redundant and can
conflict with native button behavior; remove the onKeyDown={(e) => { if (e.key
=== "Enter" || e.key === " ") { e.preventDefault(); setOpen(v => !v) } }}
handler and rely on the existing onClick handler to toggle setOpen, and while
editing confirm the element is an actual <button> (or add type="button") so
keyboard activation and form behavior remain correct.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e3ff03ac-b2b0-4340-9191-4c7bb6786bdc
⛔ Files ignored due to path filters (4)
docs/network/kratos/quickstarts/intro.mdxis excluded by!**/*.mdxdocs/oss/getting-started/index.mdxis excluded by!**/*.mdxdocs/oss/projects.mdxis excluded by!**/*.mdxsrc/components/Shared/kratos/index.mdxis excluded by!**/*.mdx
📒 Files selected for processing (5)
docusaurus.config.tssidebars-network.tssrc/components/QuickStarts/constants.tssrc/components/SidebarDeploymentModel/index.tsxsrc/pages/_assets/examples-content.tsx
✅ Files skipped from review due to trivial changes (2)
- sidebars-network.ts
- src/components/QuickStarts/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/_assets/examples-content.tsx
vinckr
left a comment
vinckr
left a comment
There was a problem hiding this comment.
The biggest issue I have with this update is that all the concept and guides docs (that apply universally across deployment options) are missing from the OSS and OEL deployment sections.
For example I should be able to find the concepts when I select OSS or OEL as deployment option because these docs are central to understanding anything about how Ory Kratos works.
Right now you can only find them if you select Ory Network as deployment option.
When I come to the quickstart - which many of our users do through GitHub - it seems like there are hardly any docs for Ory Kratos, just some for edgecases like Advanced Cookie Settinsg. Same for OEL.
All the guides and concept docs need to be in a central, easily accessible place otherwise you are completely lost.
Like I would have no idea how a login flow works with Ory Kratos when I self-host without digging into the Ory Network deployment option.
This is for me the biggest blocker to getting this merged.
There are other minor things, that we can also polish later on, but IMHO we need to have a better solution here first.
| items: [ | ||
| { type: "ref", id: "oel/kratos/intro" }, | ||
| { type: "ref", id: "kratos/install" }, | ||
| "self-hosted/oel/kratos/upgrade", |
There was a problem hiding this comment.
this page (https://docs-rfbjzxqwi-ory.vercel.app/docs/self-hosted/oel/kratos/upgrade) should be in the OEL section not OSS?
| { type: "ref", id: "oel/kratos/intro" }, | ||
| { type: "ref", id: "kratos/install" }, | ||
| "self-hosted/oel/kratos/upgrade", | ||
| "self-hosted/oel/kratos/changelog", |
There was a problem hiding this comment.
this doesn't really make sense in OSS section since it's not about the OSS releases.
There was a problem hiding this comment.
Can you specify what 'this' refers to? Is it all 4 lines 55 to 58?
| items: [ | ||
| "oel/hydra/index", | ||
| "hydra/self-hosted/install", | ||
| "self-hosted/oel/oauth2/migrate", |
There was a problem hiding this comment.
this page (https://docs-rfbjzxqwi-ory.vercel.app/docs/self-hosted/oel/oauth2/upgrade) should be in OEL section not OSS
| "self-hosted/oel/oauth2/revert-database-migrations", | ||
| "self-hosted/oel/oauth2/upgrade", | ||
| "hydra/self-hosted/upgrade", | ||
| "self-hosted/oel/oauth2/changelog", |
There was a problem hiding this comment.
this doesn't really make sense in OSS section since it's not about the OSS releases.
There was a problem hiding this comment.
Can you specify what 'this' refers to? Is it all 4 lines 137 to 140?
| "keto/guides/migrating-legacy-policies", | ||
| "keto/guides/upgrade", | ||
| "oel/keto/quickstart", | ||
| "self-hosted/oel/keto/changelog", |
There was a problem hiding this comment.
this doesn't really make sense in OSS section since it's not about the OSS releases.
There was a problem hiding this comment.
Can you specify what 'this' refers to? Is it all 4 lines 208 to 211?
| "oel/oathkeeper/index", | ||
| "oathkeeper/install", | ||
| "self-hosted/oel/oathkeeper/upgrade-oathkeeper", | ||
| "self-hosted/oel/oathkeeper/changelog", |
There was a problem hiding this comment.
this doesn't really make sense in OSS section since it's not about the OSS releases.
There was a problem hiding this comment.
Can you specify what 'this' refers to? Is it all 4 lines 427 to 430?
@vincent, the agreed upon strategy was to build out a frame work so that each deployment could have exactly only the articles relevant to it. What this release does is provide that framework. Due to SEO risk, we were not able to do everything in one release. This has the minimum changes. Future releases will build out the OEL and OSS sections. |
All of these file with 404s had a redirect which is why they weren't working. The redirects are now removed. You might not to clear browser cache to see them working or open via Cognito mode |
… in operations guide
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/welcomePage/SolutionDesignStepper.tsx`:
- Around line 163-170: The answer buttons are visually stateful but lack an ARIA
state; update the button element rendered inside SolutionDesignStepper so it
exposes its selection for assistive tech by adding an aria-pressed attribute
whose value is the same boolean used to compute selected (the same variable used
in the className), keeping the onClick that calls
handleAnswer(currentQuestion.id, option.value) unchanged; ensure the attribute
is present on the <button> that maps option.value so screen readers announce
pressed/unpressed.
- Around line 33-38: The persisted stepper state loaded by useEffect via
loadSolutionDesignProgressFromSessionStorage may contain a stale or out-of-range
currentStep which causes STEPS[currentStep].id to throw; before calling
setCurrentStep and setAnswers, validate that progress.currentStep is a safe
integer within 0 and STEPS.length - 1 (or clamp to that range) and only apply
progress.answers if it matches expected shape/length for STEPS; otherwise ignore
the persisted stepper state (or reset to default step 0) to avoid runtime errors
in SolutionDesignStepper.tsx.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c4f3031-8029-4174-a7a6-f08a0690216b
⛔ Files ignored due to path filters (1)
docs/guides/operations.mdxis excluded by!**/*.mdx
📒 Files selected for processing (1)
src/components/welcomePage/SolutionDesignStepper.tsx
| useEffect(() => { | ||
| const progress = loadSolutionDesignProgressFromSessionStorage() | ||
| if (progress) { | ||
| setAnswers(progress.answers) | ||
| setCurrentStep(progress.currentStep) | ||
| } |
There was a problem hiding this comment.
Validate persisted stepper state before applying it.
Session data can be stale/corrupted. If currentStep is out of range, the flow can get stuck and Back can throw at Line 96 (STEPS[currentStep].id).
Proposed fix
useEffect(() => {
const progress = loadSolutionDesignProgressFromSessionStorage()
if (progress) {
- setAnswers(progress.answers)
- setCurrentStep(progress.currentStep)
+ const clampedStep = Math.min(
+ Math.max(progress.currentStep ?? 0, 0),
+ STEPS.length,
+ )
+ const validStepIds = new Set(STEPS.map((s) => s.id))
+ const sanitizedAnswers = Object.fromEntries(
+ Object.entries(progress.answers ?? {}).filter(
+ ([id, value]) =>
+ validStepIds.has(id) && (value === "yes" || value === "no"),
+ ),
+ ) as Record<string, "yes" | "no">
+
+ setAnswers(sanitizedAnswers)
+ setCurrentStep(clampedStep)
}
}, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/welcomePage/SolutionDesignStepper.tsx` around lines 33 - 38,
The persisted stepper state loaded by useEffect via
loadSolutionDesignProgressFromSessionStorage may contain a stale or out-of-range
currentStep which causes STEPS[currentStep].id to throw; before calling
setCurrentStep and setAnswers, validate that progress.currentStep is a safe
integer within 0 and STEPS.length - 1 (or clamp to that range) and only apply
progress.answers if it matches expected shape/length for STEPS; otherwise ignore
the persisted stepper state (or reset to default step 0) to avoid runtime errors
in SolutionDesignStepper.tsx.
| <button | ||
| key={option.value} | ||
| type="button" | ||
| onClick={() => | ||
| handleAnswer(currentQuestion.id, option.value) | ||
| } | ||
| className={`w-full flex items-center text-left bg-ory-bg-primary border rounded-ory-btn py-ory-2 px-ory-4 ory-body-sm cursor-pointer text-ory-text-primary ${selected ? "border-ory-border-brand-tertiary" : "border-ory-border-primary"}`} | ||
| > |
There was a problem hiding this comment.
Expose selected state for answer buttons to assistive tech.
These buttons are visually stateful, but selection is not announced. Add an ARIA pressed state.
Proposed fix
<button
key={option.value}
type="button"
+ aria-pressed={selected}
onClick={() =>
handleAnswer(currentQuestion.id, option.value)
}
className={`w-full flex items-center text-left bg-ory-bg-primary border rounded-ory-btn py-ory-2 px-ory-4 ory-body-sm cursor-pointer text-ory-text-primary ${selected ? "border-ory-border-brand-tertiary" : "border-ory-border-primary"}`}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| key={option.value} | |
| type="button" | |
| onClick={() => | |
| handleAnswer(currentQuestion.id, option.value) | |
| } | |
| className={`w-full flex items-center text-left bg-ory-bg-primary border rounded-ory-btn py-ory-2 px-ory-4 ory-body-sm cursor-pointer text-ory-text-primary ${selected ? "border-ory-border-brand-tertiary" : "border-ory-border-primary"}`} | |
| > | |
| <button | |
| key={option.value} | |
| type="button" | |
| aria-pressed={selected} | |
| onClick={() => | |
| handleAnswer(currentQuestion.id, option.value) | |
| } | |
| className={`w-full flex items-center text-left bg-ory-bg-primary border rounded-ory-btn py-ory-2 px-ory-4 ory-body-sm cursor-pointer text-ory-text-primary ${selected ? "border-ory-border-brand-tertiary" : "border-ory-border-primary"}`} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/welcomePage/SolutionDesignStepper.tsx` around lines 163 - 170,
The answer buttons are visually stateful but lack an ARIA state; update the
button element rendered inside SolutionDesignStepper so it exposes its selection
for assistive tech by adding an aria-pressed attribute whose value is the same
boolean used to compute selected (the same variable used in the className),
keeping the onClick that calls handleAnswer(currentQuestion.id, option.value)
unchanged; ensure the attribute is present on the <button> that maps
option.value so screen readers announce pressed/unpressed.
| sidebar_label: Ory Keto Quickstart | ||
| --- | ||
|
|
||
| <head> |
There was a problem hiding this comment.
All of these files that have MDX need the mdx code fence, see readme: https://github.com/ory/docs?tab=readme-ov-file#import--reference-content
17 participants
Preview link: https://docs-rfbjzxqwi-ory.vercel.app/docs
Summary
This PR delivers a significant redesign of the Ory documentation site, introducing a new Homepage navigation experience, a new Quick Start page, restructured content architecture, and visual design improvements. It also establishes the foundational framework for separating documentation by deployment model: Ory Network, Ory OEL, and Ory OSS.
Changes
Homepage navigation
Replaced the previous homepage with a guided navigation experience that helps technical evaluators and developers orient quickly and find the right entry point for their use case. (Aligns better with our marketing site.)
Quick Start page
Added a new Quick Start page as a dedicated onboarding path for developers getting started with Ory for the first time.
Content restructuring
Reorganized existing documentation to support clearer content hierarchy. This restructuring is a prerequisite for the deployment-model separation work.
Product overview
Added new high-level product explanations with visual elements to demonstrate how each product fits within the context of an IAM system.
Visual design enhancements
Updated layout, typography, and component styling throughout the portal for improved readability and consistency. Introduced product color association.
Deployment model framework
Introduced the structural scaffolding to separate documentation by deployment context — Ory Network, Ory OEL, and Ory OSS — enabling product-specific content targeting in follow-on PRs.
What's not included
Full content separation by deployment model is out of scope for this PR. This change puts the framework in place; content migration and product-specific pages will follow.
Complete alignment on product names is out of the scope of this PR.
TESTING
New feature documentation (last few months)
Verify your new content appears in the redesigned navigation (docs/sidebar.ts). If it doesn't appear, add it to the /docs/sidebar.ts.
NOTE: There are several new sidebar files located within the /docs folder; if your content was added to the previous sidebar.ts file located in /src that file is no longer in use.
Content is complete
Verify that your product area contains the expected full set of content. The restructure may have affected navigation visibility for some pages.
Broken links
Spot-check links in your product area. Internal navigation files were significantly changed in this redesign.
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.com) from the maintainers to push the changes.
Summary by CodeRabbit
New Features
Documentation
Updates