-
Notifications
You must be signed in to change notification settings - Fork 5
7.0.3 fixes #1516
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
7.0.3 fixes #1516
Conversation
Fix sentry dsn client key not being in window env Fix auth link building Refactor Tabs to be more stable and simple Fix package and wiki pages data loading on demand
WalkthroughIntroduces wiki support across package pages with asynchronous loaders, Suspense/Await rendering, and permission-gated UI. Refactors Tabs to children-based API with accompanying CSS selector updates. Adopts public environment variables for auth URLs and Sentry DSN. Removes client middleware/context file. Adds isPromise utility. Multiple components updated to new Tabs and auth patterns. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant R as Remix Router
participant L as loader()/clientLoader()
participant D as Dapper API
participant UI as Wiki UI (Suspense/Await)
U->>R: Navigate to Package Wiki route
R->>L: Invoke loader (SSR) or clientLoader
par Fetch data
L->>D: getPackageWiki(namespaceId, packageId)
L-->>L: If 404 → wiki = undefined
L->>D: getPackageWikiPage(wikiId, slug?) (page routes)
L-->>L: If 404 → page = undefined
L->>D: getPackagePermissions(...)
end
L-->>R: Return { wiki?, page?, permissions (or Promise) }
R->>UI: Render with Suspense/Await
UI->>UI: Resolve permissions → can_manage
alt wiki/page exist
UI-->>U: Render WikiContent, compute prev/next client-side
opt can_manage
UI-->>U: Show "New Page" / "Edit" actions
end
else not found
UI-->>U: Show fallback ("No pages"/"Page Not Found")
end
sequenceDiagram
autonumber
participant C as Component (Nav/Connections)
participant E as getPublicEnvVariables
participant A as buildAuthLoginUrl
C->>E: Read VITE_AUTH_BASE_URL, VITE_AUTH_RETURN_URL
E-->>C: { authBaseDomain, authReturnDomain }
C->>A: buildAuthLoginUrl({ type, authBaseDomain, authReturnDomain, nextUrl? })
A-->>C: Full auth URL with ?next=
C-->>C: Render provider login link
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
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.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx (1)
55-56
: Replace "TODO" in success toast with meaningful provider textEnd-user string ships with "TODO". At minimum, make it generic; ideally include the provider name.
Apply this minimal fix now, and consider the follow-up below for provider-specific text:
- User {outletContext.currentUser.username} was disconnected from TODO + User {outletContext.currentUser.username} was disconnected from their accountFollow-up (optional, preferred): capture provider in the success handler by creating the ApiAction per provider inside the map so the toast can say “…disconnected from Discord/GitHub/Overwolf”.
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx (1)
170-181
: Fix next/previous index checks and handle “page not in list”.Off-by-one in nextPage and no guard for -1 index.
- if (currentPageIndex === 0) { + if (currentPageIndex <= 0) { previousPage = undefined; } else { previousPage = wiki.pages[currentPageIndex - 1]?.slug; } - if (currentPageIndex === wiki.pages.length) { + if (currentPageIndex < 0 || currentPageIndex >= wiki.pages.length - 1) { nextPage = undefined; } else { nextPage = wiki.pages[currentPageIndex + 1]?.slug; }apps/cyberstorm-remix/app/p/packageListing.tsx (1)
51-56
: Don’t create promises inside render; memoize derived access check.Creating a new withResolvers promise during render can cause flicker and leaks. Derive a stable promise with useMemo and await wiki/permissions safely.
import { ReactElement, Suspense, useEffect, useReducer, useRef, useState, + useMemo, } from "react";
- const canAccessWikiTabPromise = Promise.withResolvers<boolean>(); - if (wiki && isPromise(wiki)) { - wiki - .then(async (a) => { - if (a.pages.length > 0) { - canAccessWikiTabPromise.resolve(true); - } else { - if ((await permissions)?.permissions.can_manage) { - canAccessWikiTabPromise.resolve(true); - } else { - canAccessWikiTabPromise.resolve(false); - } - } - }) - .catch(async () => { - if ((await permissions)?.permissions.can_manage) { - canAccessWikiTabPromise.resolve(true); - } else { - canAccessWikiTabPromise.resolve(false); - } - }); - } else { - if (permissions) { - permissions.then((a) => { - if (a?.permissions.can_manage) { - canAccessWikiTabPromise.resolve(true); - } else { - canAccessWikiTabPromise.resolve(false); - } - }); - } else { - canAccessWikiTabPromise.resolve(false); - } - } + const canAccessWikiTabPromise = useMemo(() => { + return (async () => { + try { + const w = isPromise(wiki) ? await wiki : wiki; + if (w && w.pages.length > 0) return true; + } catch { + // ignore wiki errors here; fall back to permissions + } + try { + const p = permissions ? await permissions : undefined; + return Boolean(p?.permissions.can_manage); + } catch { + return false; + } + })(); + }, [wiki, permissions]);- resolve={canAccessWikiTabPromise.promise} + resolve={canAccessWikiTabPromise}Also applies to: 567-600, 784-784
🧹 Nitpick comments (23)
apps/cyberstorm-remix/app/entry.client.tsx (2)
10-16
: Guard Sentry init when DSN is absent.Prevents noisy init and aligns with env-driven runtime config.
-const sentryClientDSN = getPublicEnvVariables([ - "VITE_CLIENT_SENTRY_DSN", -]).VITE_CLIENT_SENTRY_DSN; +const sentryClientDSN = getPublicEnvVariables([ + "VITE_CLIENT_SENTRY_DSN", +]).VITE_CLIENT_SENTRY_DSN;
14-38
: Wrap Sentry initialization in a DSN presence check-Sentry.init({ - dsn: sentryClientDSN, - integrations: [ - Sentry.browserTracingIntegration({ useEffect, useLocation, useMatches }), - // Replay is only available in the client - Sentry.replayIntegration(), - ], - tracesSampleRate: 0, - tracePropagationTargets: [], - replaysSessionSampleRate: 0, - replaysOnErrorSampleRate: 0, -}); +if (sentryClientDSN) { + Sentry.init({ + dsn: sentryClientDSN, + integrations: [ + Sentry.browserTracingIntegration({ useEffect, useLocation, useMatches }), + // Replay is only available in the client + Sentry.replayIntegration(), + ], + tracesSampleRate: 0, + tracePropagationTargets: [], + replaysSessionSampleRate: 0, + replaysOnErrorSampleRate: 0, + }); +}apps/cyberstorm-remix/cyberstorm/utils/ThunderstoreAuth.tsx (1)
9-13
: Harden URL building: trim trailing slashes to avoid//
and reduce redirects.-export function buildAuthLoginUrl(props: Props) { - return `${props.authBaseDomain}/auth/login/${props.type}/${ - props.nextUrl - ? `?next=${encodeURIComponent(props.nextUrl)}` - : `?next=${encodeURIComponent(`${props.authReturnDomain}/communities/`)}` - }`; -} +export function buildAuthLoginUrl(props: Props) { + const base = props.authBaseDomain.replace(/\/+$/, ""); + const returnBase = props.authReturnDomain.replace(/\/+$/, ""); + return `${base}/auth/login/${props.type}/${ + props.nextUrl + ? `?next=${encodeURIComponent(props.nextUrl)}` + : `?next=${encodeURIComponent(`${returnBase}/communities/`)}` + }`; +}Please confirm trailing-slash behavior matches existing auth endpoints (some backends 301 on
//
).apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx (3)
230-234
: DRY: compute public env once and reuse.Both DesktopLoginPopover and MobileUserPopoverContent fetch the same vars; hoist to a small helper/hook to avoid duplication.
Also applies to: 541-545
290-294
: Guard OAuth links when env is missing.If either
VITE_AUTH_BASE_URL
orVITE_AUTH_RETURN_URL
is empty, consider hiding/disabled buttons to avoid dead links.Also applies to: 304-308, 318-322, 626-630, 640-644, 654-658
314-314
: Fix brand casing: “GitHub”.- Connect with Github + Connect with GitHubAlso applies to: 651-651
apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx (2)
63-70
: Make tab detection robust to trailing slashes and nested pathsendsWith can fail with trailing slashes; derive the last pathname segment instead.
Apply:
- const currentTab = location.pathname.endsWith("/settings") - ? "settings" - : location.pathname.endsWith("/members") - ? "members" - : location.pathname.endsWith("/service-accounts") - ? "service-accounts" - : "profile"; + const last = location.pathname.replace(/\/+$/, "").split("/").pop(); + const currentTab = + last === "settings" || last === "members" || last === "service-accounts" + ? (last as "settings" | "members" | "service-accounts") + : "profile";
90-137
: Remove unnecessary React keys on static childrenKeys aren’t needed when not rendering a list via map; removing reduces noise.
Apply:
- <NewLink - key="members" + <NewLink primitiveType="cyberstormLink"Repeat for the other three NewLink elements.
apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx (3)
114-119
: Return users to the current page after auth linkingIncluding
nextUrl
ensures users land back on Connections instead of a generic default.connectionLink={buildAuthLoginUrl({ type: p.identifier, authBaseDomain: publicEnvVariables.VITE_AUTH_BASE_URL || "", authReturnDomain: - publicEnvVariables.VITE_AUTH_RETURN_URL || "", + publicEnvVariables.VITE_AUTH_RETURN_URL ?? + (typeof window !== "undefined" ? window.location.origin : ""), + nextUrl: + typeof window !== "undefined" ? window.location.href : undefined, })}
32-35
: Nit: Brand casing“Github” → “GitHub”.
- name: "Github", + name: "GitHub",
100-105
: Redundant not-logged-in checks inside mapYou already return earlier. These throws are defensive but unnecessary noise.
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx (1)
52-76
: Add Suspense fallback and Await errorElement to avoid UI flicker/crashWithout a fallback, the action area pops in; without errorElement, a rejected promise may bubble to an error boundary.
- <Suspense> - <Await resolve={canManage}> + <Suspense fallback={null}> + <Await resolve={canManage} errorElement={<></>}>apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx (2)
144-168
: Improve a11y for Tabs: roles/ARIA; remove unnecessary keys; set button type
- Use role="tablist"/"tab" + aria-selected.
- Remove redundant key props on static children.
- Set type="button" to avoid implicit submit in nested forms.
- <Tabs> - <button - key="write" - onClick={() => setSelectedTab("write")} - aria-current={selectedTab === "write"} + <Tabs role="tablist"> + <button + type="button" + role="tab" + onClick={() => setSelectedTab("write")} + aria-selected={selectedTab === "write"} className={classnames( "wiki-edit__tabs-button", "tabs-item", selectedTab === "write" ? "tabs-item--current" : undefined )} > Write </button> - <button - key="preview" - onClick={() => setSelectedTab("preview")} - aria-current={selectedTab === "preview"} + <button + type="button" + role="tab" + onClick={() => setSelectedTab("preview")} + aria-selected={selectedTab === "preview"} className={classnames( "wiki-edit__tabs-button", "tabs-item", selectedTab === "preview" ? "tabs-item--current" : undefined )} > Preview </button> </Tabs>Additionally, wrap panels with role="tabpanel" and link via aria-labelledby:
{selectedTab === "write" ? ( <div role="tabpanel" aria-labelledby="tab-write"> {/* existing write content */} </div> ) : ( <div role="tabpanel" aria-labelledby="tab-preview"> <Markdown input={formInputs.markdown_content} /> </div> )}
38-48
: DRY loadersclientLoader duplicates loader verbatim.
-export async function clientLoader({ params }: LoaderFunctionArgs) { - if (params.communityId && params.namespaceId && params.packageId) { - return { - communityId: params.communityId, - namespaceId: params.namespaceId, - packageId: params.packageId, - }; - } else { - throw new Error("Namespace ID or Package ID is missing"); - } -} +export const clientLoader = loader;apps/cyberstorm-remix/app/settings/user/Settings.tsx (2)
60-83
: Adopt tab semantics (role=tablist/role=tab) and avoid boolean aria-current.Improves a11y; use role and aria-current="page" only when active.
- <Tabs> + <Tabs role="tablist"> <NewLink key="settings" primitiveType="cyberstormLink" linkId="Settings" - aria-current={currentTab === "settings"} + role="tab" + aria-current={currentTab === "settings" ? "page" : undefined} rootClasses={`tabs-item${ currentTab === "settings" ? " tabs-item--current" : "" }`} > Settings </NewLink> <NewLink key="account" primitiveType="cyberstormLink" linkId="SettingsAccount" - aria-current={currentTab === "account"} + role="tab" + aria-current={currentTab === "account" ? "page" : undefined} rootClasses={`tabs-item${ currentTab === "account" ? " tabs-item--current" : "" }`} > Account </NewLink> </Tabs>
60-83
: Unnecessary keys on static children.Keys aren’t needed when not mapping arrays.
- <NewLink - key="settings" + <NewLink primitiveType="cyberstormLink" linkId="Settings" @@ - <NewLink - key="account" + <NewLink primitiveType="cyberstormLink" linkId="SettingsAccount"apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (1)
238-263
: Use proper tab semantics; replace aria-current with aria-selected and set role.Improves accessibility and avoids misleading aria-current usage on buttons.
- <Tabs> + <Tabs role="tablist"> - <button - key="write" - onClick={() => setSelectedTab("write")} - aria-current={selectedTab === "write"} + <button + type="button" + role="tab" + onClick={() => setSelectedTab("write")} + aria-selected={selectedTab === "write"} className={classnames( "wiki-edit__tabs-button", "tabs-item", selectedTab === "write" ? "tabs-item--current" : undefined )} > Write </button> - <button - key="preview" - onClick={() => setSelectedTab("preview")} - aria-current={selectedTab === "preview"} + <button + type="button" + role="tab" + onClick={() => setSelectedTab("preview")} + aria-selected={selectedTab === "preview"} className={classnames( "wiki-edit__tabs-button", "tabs-item", selectedTab === "preview" ? "tabs-item--current" : undefined )} > Preview </button> </Tabs>apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (1)
161-164
: Always pass a Promise to WikiContent.canManage.Ensures stable type on SSR (when permissions is undefined).
- canManage={permissions?.then((perms) => - typeof perms === "undefined" ? false : perms.permissions.can_manage - )} + canManage={ + permissions + ? permissions.then( + (perms) => perms?.permissions.can_manage ?? false + ) + : Promise.resolve(false) + }apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx (2)
38-46
: Don’t silently swallow non-404 errors in the server loader.Returning a normal page while hiding backend issues makes debugging hard. Consider rethrowing non-404s so Remix error boundaries surface them.
Example:
- } else { - wiki = undefined; - console.error("Error fetching package wiki:", error); - } + } else { + throw error; + }
101-122
: Add errorElement and a minimal fallback for permissions Await.Prevents unhandled rejections and avoids an empty flash if the promise rejects.
- <Suspense> - <Await resolve={permissions}> + <Suspense fallback={null}> + <Await resolve={permissions} errorElement={null}> {(resolvedValue) => resolvedValue?.permissions.can_manage ? (apps/cyberstorm-remix/app/p/packageListing.tsx (3)
189-205
: Simplify permissions promise; drop withResolvers wrapper.No need to proxy the promise—return the original when logged in.
- // We do some trickery right here to prevent unnecessary request when the user is not logged in - let permissionsPromise = undefined; - const cu = await tools.getSessionCurrentUser(); - if (cu.username) { - const wrapperPromise = - Promise.withResolvers< - Awaited<ReturnType<typeof getPackagePermissions>> - >(); - dapper - .getPackagePermissions( - params.communityId, - params.namespaceId, - params.packageId - ) - .then(wrapperPromise.resolve, wrapperPromise.reject); - permissionsPromise = wrapperPromise.promise; - } + // Avoid request if not logged in + let permissionsPromise = undefined; + const cu = await tools.getSessionCurrentUser(); + if (cu.username) { + permissionsPromise = dapper.getPackagePermissions( + params.communityId, + params.namespaceId, + params.packageId + ); + }
149-160
: Don’t swallow non-404 wiki errors in the server loader.Surface real failures to error boundaries; only 404 should be treated as “no wiki”.
- } catch (error) { - if (error instanceof ApiError) { - if (error.response.status === 404) { - wiki = undefined; - } else { - wiki = undefined; - console.error("Error fetching package wiki:", error); - } - } - } + } catch (error) { + if (error instanceof ApiError && error.response.status === 404) { + wiki = undefined; + } else { + throw error; + } + }
162-173
: Parallelize independent loader calls for latency reduction.These requests don’t depend on each other.
- return { - community: await dapper.getCommunity(params.communityId), - communityFilters: await dapper.getCommunityFilters(params.communityId), - listing: await dapper.getPackageListingDetails( - params.communityId, - params.namespaceId, - params.packageId - ), - team: await dapper.getTeamDetails(params.namespaceId), - permissions: undefined, - wiki: wiki, - }; + const [community, communityFilters, listing, team] = await Promise.all([ + dapper.getCommunity(params.communityId), + dapper.getCommunityFilters(params.communityId), + dapper.getPackageListingDetails( + params.communityId, + params.namespaceId, + params.packageId + ), + dapper.getTeamDetails(params.namespaceId), + ]); + return { community, communityFilters, listing, team, permissions: undefined, wiki };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx
(9 hunks)apps/cyberstorm-remix/app/entry.client.tsx
(1 hunks)apps/cyberstorm-remix/app/middlewares.ts
(0 hunks)apps/cyberstorm-remix/app/p/packageListing.tsx
(5 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx
(5 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx
(3 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx
(3 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx
(1 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx
(4 hunks)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx
(2 hunks)apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx
(1 hunks)apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx
(3 hunks)apps/cyberstorm-remix/app/settings/user/Settings.tsx
(1 hunks)apps/cyberstorm-remix/cyberstorm/utils/ThunderstoreAuth.tsx
(1 hunks)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts
(1 hunks)packages/cyberstorm-theme/src/components/Tabs/Tabs.css
(1 hunks)packages/cyberstorm/src/newComponents/Tabs/Tabs.css
(2 hunks)packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx
(2 hunks)
💤 Files with no reviewable changes (1)
- apps/cyberstorm-remix/app/middlewares.ts
🧰 Additional context used
🧬 Code graph analysis (12)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx (2)
packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx (1)
Tabs
(15-37)packages/cyberstorm/src/utils/utils.ts (1)
classnames
(34-38)
apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx (2)
packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx (1)
Tabs
(15-37)packages/cyberstorm/src/index.ts (1)
Tabs
(93-93)
apps/cyberstorm-remix/app/entry.client.tsx (1)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
getPublicEnvVariables
(17-35)
apps/cyberstorm-remix/app/settings/user/Settings.tsx (1)
packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx (1)
Tabs
(15-37)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (3)
packages/dapper-ts/src/methods/package.ts (3)
getPackageWiki
(84-100)getPackageWikiPage
(102-113)getPackagePermissions
(157-183)packages/thunderstore-api/src/errors.ts (1)
isApiError
(10-12)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx (1)
WikiContent
(26-138)
apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
publicEnvVariables
(13-15)getPublicEnvVariables
(17-35)apps/cyberstorm-remix/cyberstorm/utils/ThunderstoreAuth.tsx (1)
buildAuthLoginUrl
(8-14)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (2)
packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx (1)
Tabs
(15-37)packages/cyberstorm/src/utils/utils.ts (1)
classnames
(34-38)
apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx (2)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (2)
publicEnvVariables
(13-15)getPublicEnvVariables
(17-35)apps/cyberstorm-remix/cyberstorm/utils/ThunderstoreAuth.tsx (1)
buildAuthLoginUrl
(8-14)
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx (4)
packages/dapper-ts/src/methods/package.ts (3)
getPackageWiki
(84-100)getPackageWikiPage
(102-113)getPackagePermissions
(157-183)packages/thunderstore-api/src/errors.ts (1)
isApiError
(10-12)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (1)
loader
(27-82)apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx (1)
WikiContent
(26-138)
apps/cyberstorm-remix/app/p/packageListing.tsx (5)
apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (3)
publicEnvVariables
(13-15)getPublicEnvVariables
(17-35)getSessionTools
(37-54)packages/dapper-ts/src/index.ts (1)
DapperTs
(35-86)packages/dapper-ts/src/methods/package.ts (2)
getPackageWiki
(84-100)getPackagePermissions
(157-183)packages/thunderstore-api/src/errors.ts (1)
ApiError
(14-64)apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts (1)
isPromise
(5-7)
apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx (2)
packages/dapper-ts/src/methods/package.ts (1)
getPackageWiki
(84-100)packages/thunderstore-api/src/errors.ts (1)
ApiError
(14-64)
packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx (2)
packages/cyberstorm/src/index.ts (2)
FrameWindowProps
(31-31)Tabs
(93-93)packages/cyberstorm-theme/src/components/Tabs/Tabs.ts (2)
TabsVariants
(3-3)TabsSizes
(7-7)
🪛 GitHub Check: ESLint
apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts
[failure] 5-5: Disallow the any
type
Unexpected any. Specify a different type.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test
- GitHub Check: Generate visual diffs
- GitHub Check: CodeQL
🔇 Additional comments (10)
packages/cyberstorm/src/newComponents/Tabs/Tabs.css (2)
35-44
: LGTM: number badge rename aligns with TSX changes.
12-33
: No legacy selectors detected: I verified across the codebase and found no remaining references to.tabs__item
,.tabs__item--current
, or.tabs__number-slate
.apps/cyberstorm-remix/app/commonComponents/Navigation/Navigation.tsx (1)
42-43
: LGTM: runtime env retrieval aligns with new auth URL builder.packages/cyberstorm-theme/src/components/Tabs/Tabs.css (2)
9-26
: Sizes block restructure looks solidScoped variables per size under the parent is cleaner and easier to reason about.
9-94
: Confirm CSS nesting support in the buildThis file relies on native CSS nesting (&, nested selectors). Ensure your build (e.g., Lightning CSS/PostCSS nesting) or browser support matrix guarantees correct output.
If needed, I can scan your config files; just share where PostCSS/Lightning CSS is configured.
packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx (1)
15-37
: Children-based Tabs API LGTMSimpler surface, fewer props, and easier composition.
apps/cyberstorm-remix/app/settings/teams/team/teamSettings.tsx (1)
90-138
: Verifyteam
prop matches the route slug
Theteam
prop drives the/teams/${team}
URL; passingteam.name
will break navigation unlessname
is the slug—useteam.slug
(or whatever field your route expects) instead ofteam.name
.apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx (1)
12-15
: Prop type change to Promise: verify all call sites updatedEnsure all usages of WikiContent now pass a Promise for canManage.
Would you like a repo scan to list all <WikiContent … canManage=…> call sites and flag non-promise values?
Also applies to: 23-24
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (1)
107-107
: Rename OK.Default export rename to WikiEdit is fine; no route breakage.
apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (1)
60-74
: Clarify error handling for permissions.Comment mentions “no permission” but only 404 is handled. Consider 401/403 if API returns those for restricted wikis.
Would you like me to update the catcher to also treat 401/403 as “no wiki visible”?
import { Suspense } from "react"; | ||
import { ApiError } from "../../../../../../packages/thunderstore-api/src"; | ||
import { getPackageWiki } from "@thunderstore/dapper-ts/src/methods/package"; | ||
|
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.
🛠️ Refactor suggestion
Avoid instanceof traps and deep imports; use isApiError and method ReturnType off the class.
Mixing a relative import of ApiError with package imports can break instanceof checks at runtime. Also, importing types from deep paths is brittle. Switch to isApiError from the package and rely on DapperTs['getPackageWiki'] for typing.
Apply:
import { Suspense } from "react";
-import { ApiError } from "../../../../../../packages/thunderstore-api/src";
-import { getPackageWiki } from "@thunderstore/dapper-ts/src/methods/package";
+import { isApiError } from "@thunderstore/thunderstore-api";
- let wiki: Awaited<ReturnType<typeof getPackageWiki>> | undefined;
+ let wiki: Awaited<ReturnType<DapperTs["getPackageWiki"]>> | undefined;
- } catch (error) {
- if (error instanceof ApiError) {
- if (error.response.status === 404) {
- wiki = undefined;
- } else {
- wiki = undefined;
- console.error("Error fetching package wiki:", error);
- }
- }
- }
+ } catch (error) {
+ if (isApiError(error)) {
+ if (error.response.status === 404) {
+ wiki = undefined;
+ } else {
+ wiki = undefined;
+ console.error("Error fetching package wiki:", error);
+ }
+ } else {
+ throw error;
+ }
+ }
Also applies to: 33-33, 35-46
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.tsx around lines 19-22 (and also
apply same changes at 33 and 35-46), avoid importing ApiError via a deep
relative path and avoid instanceof checks; replace that with the
package-provided isApiError predicate and remove deep imports of methods/types.
Import the public isApiError from the thunderstore-api package, import
getPackageWiki from the public dapper-ts entry, and type the wiki response using
ReturnType<typeof DapperTs['getPackageWiki']> (or the equivalent exported client
type) instead of directly importing class/type from deep paths; update error
handling to use isApiError(err) and read error details from the typed error
object.
import { isApiError } from "../../../../../../packages/thunderstore-api/src"; | ||
import { | ||
getPackagePermissions, | ||
getPackageWiki, | ||
getPackageWikiPage, | ||
} from "@thunderstore/dapper-ts/src/methods/package"; | ||
|
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.
🛠️ Refactor suggestion
Avoid deep src imports; use public entrypoints.
Deep imports break bundling and API boundaries.
-import { isApiError } from "../../../../../../packages/thunderstore-api/src";
-import {
- getPackagePermissions,
- getPackageWiki,
- getPackageWikiPage,
-} from "@thunderstore/dapper-ts/src/methods/package";
+import { isApiError } from "@thunderstore/thunderstore-api";
Committable suggestion skipped: line range outside the PR's diff.
💡 Verification agent
🧩 Analysis chain
Repo-wide scan for private “/src” imports (automated).
Ensures no other deep-imports slipped in.
🏁 Script executed:
#!/bin/bash
# Find deep /src imports of internal packages that can break builds
rg -nP -C1 '(packages\/thunderstore-[^\/]+\/src|@thunderstore\/(?:dapper-ts|thunderstore-api)\/src)' --glob '!**/dist/**' --glob '!**/build/**'
Length of output: 4302
Replace deep “/src” imports with public entrypoints
Deep imports under @thunderstore/*/src/...
and packages/thunderstore-*/src/...
were found and must be refactored to use the package’s root exports. For example:
File: apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx (Lines 11–17)
import { isApiError } from "../../../../../../packages/thunderstore-api/src";
import {
getPackagePermissions,
getPackageWikiPage,
} from "@thunderstore/dapper-ts/src/methods/package";
Should become:
import { isApiError } from "@thunderstore/thunderstore-api";
import { getPackagePermissions, getPackageWikiPage } from "@thunderstore/dapper-ts";
Apply the same change to all occurrences reported by the scan script.
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx around lines 11 to
17, replace the deep "/src" imports with the package public entrypoints: import
isApiError from "@thunderstore/thunderstore-api" and import
getPackagePermissions/getPackageWiki/getPackageWikiPage from
"@thunderstore/dapper-ts" (or the package root exports that expose those
functions). Update all occurrences in this file to use the root package imports
rather than paths into /src so bundlers and package boundaries use the public
API.
type ResultType = { | ||
wiki: Awaited<ReturnType<typeof getPackageWiki>> | undefined; | ||
firstPage: Awaited<ReturnType<typeof getPackageWikiPage>> | undefined; | ||
communityId: string; | ||
namespaceId: string; | ||
packageId: string; | ||
permissions: ReturnType<typeof getPackagePermissions> | undefined; | ||
}; |
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.
🛠️ Refactor suggestion
Decouple types from internal functions.
Derive types from DapperTs methods instead of private paths.
-type ResultType = {
- wiki: Awaited<ReturnType<typeof getPackageWiki>> | undefined;
- firstPage: Awaited<ReturnType<typeof getPackageWikiPage>> | undefined;
- communityId: string;
- namespaceId: string;
- packageId: string;
- permissions: ReturnType<typeof getPackagePermissions> | undefined;
-};
+type ResultType = {
+ wiki:
+ | Awaited<ReturnType<InstanceType<typeof DapperTs>["getPackageWiki"]>>
+ | undefined;
+ firstPage:
+ | Awaited<
+ ReturnType<InstanceType<typeof DapperTs>["getPackageWikiPage"]>
+ >
+ | undefined;
+ communityId: string;
+ namespaceId: string;
+ packageId: string;
+ permissions:
+ | ReturnType<
+ InstanceType<typeof DapperTs>["getPackagePermissions"]
+ >
+ | undefined;
+};
📝 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.
type ResultType = { | |
wiki: Awaited<ReturnType<typeof getPackageWiki>> | undefined; | |
firstPage: Awaited<ReturnType<typeof getPackageWikiPage>> | undefined; | |
communityId: string; | |
namespaceId: string; | |
packageId: string; | |
permissions: ReturnType<typeof getPackagePermissions> | undefined; | |
}; | |
type ResultType = { | |
wiki: | |
| Awaited<ReturnType<InstanceType<typeof DapperTs>["getPackageWiki"]>> | |
| undefined; | |
firstPage: | |
| Awaited< | |
ReturnType<InstanceType<typeof DapperTs>["getPackageWikiPage"]> | |
> | |
| undefined; | |
communityId: string; | |
namespaceId: string; | |
packageId: string; | |
permissions: | |
| ReturnType< | |
InstanceType<typeof DapperTs>["getPackagePermissions"] | |
> | |
| undefined; | |
}; |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx around lines 18 to
25, the ResultType currently derives types from local internal functions
(getPackageWiki, getPackageWikiPage, getPackagePermissions); replace those with
the public types exported by DapperTs: import the appropriate exported types for
a Wiki, WikiPage (or Page) and PackagePermissions (or equivalent) from the
DapperTs package and use them for the wiki, firstPage and permissions fields
instead of Awaited<ReturnType<...>>; keep communityId, namespaceId and packageId
as strings and update the imports and type names to the exact exported
identifiers from DapperTs.
try { | ||
const wiki = await dapper.getPackageWiki( | ||
params.namespaceId, | ||
params.packageId | ||
); | ||
const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id); | ||
result = { |
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.
Guard against empty wiki.pages to prevent runtime crash.
Accessing pages[0].id can throw when pages is empty.
- const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id);
+ const firstPage =
+ wiki.pages.length > 0
+ ? await dapper.getPackageWikiPage(wiki.pages[0].id)
+ : undefined;
📝 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.
try { | |
const wiki = await dapper.getPackageWiki( | |
params.namespaceId, | |
params.packageId | |
); | |
const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id); | |
result = { | |
try { | |
const wiki = await dapper.getPackageWiki( | |
params.namespaceId, | |
params.packageId | |
); | |
const firstPage = | |
wiki.pages.length > 0 | |
? await dapper.getPackageWikiPage(wiki.pages[0].id) | |
: undefined; | |
result = { |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx around lines 45 to
51, the code assumes wiki.pages[0] exists and will crash if pages is empty;
guard by checking that wiki and Array.isArray(wiki.pages) and wiki.pages.length
> 0 before accessing pages[0].id, and handle the empty-case by returning an
appropriate fallback result (e.g., null/empty page, 404, or skip fetching the
first page) or throwing a clear error; ensure the branch avoids calling
getPackageWikiPage when there is no first page.
try { | ||
const wiki = await dapper.getPackageWiki( | ||
params.namespaceId, | ||
params.packageId | ||
); | ||
const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id); | ||
result = { |
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.
🛠️ Refactor suggestion
Repeat the empty-pages guard in client loader.
Mirror the server-side fix to keep parity.
- const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id);
+ const firstPage =
+ wiki.pages.length > 0
+ ? await dapper.getPackageWikiPage(wiki.pages[0].id)
+ : undefined;
📝 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.
try { | |
const wiki = await dapper.getPackageWiki( | |
params.namespaceId, | |
params.packageId | |
); | |
const firstPage = await dapper.getPackageWikiPage(wiki.pages[0].id); | |
result = { | |
try { | |
const wiki = await dapper.getPackageWiki( | |
params.namespaceId, | |
params.packageId | |
); | |
const firstPage = | |
wiki.pages.length > 0 | |
? await dapper.getPackageWikiPage(wiki.pages[0].id) | |
: undefined; | |
result = { |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/p/tabs/Wiki/WikiFirstPage.tsx around lines 109-115,
the client loader repeats accessing wiki.pages[0] without guarding for an empty
pages array; mirror the server-side fix by checking that wiki.pages exists and
has length > 0 before calling getPackageWikiPage, and handle the empty case the
same way the server does (e.g., return null/empty result or perform the same
redirect/404 behavior) so we never read pages[0] when the array is empty.
const publicEnvVariables = getPublicEnvVariables([ | ||
"VITE_AUTH_BASE_URL", | ||
"VITE_AUTH_RETURN_URL", | ||
]); |
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.
💡 Verification agent
🧩 Analysis chain
Harden auth URL construction: avoid empty-string domains and add safer fallbacks
Empty-string fallbacks can yield relative URLs and broken redirects. Prefer explicit origins and guard missing config.
Apply this diff to improve resilience for return domain; verify base domain is set in all envs:
- connectionLink={buildAuthLoginUrl({
+ connectionLink={buildAuthLoginUrl({
type: p.identifier,
- authBaseDomain: publicEnvVariables.VITE_AUTH_BASE_URL || "",
- authReturnDomain:
- publicEnvVariables.VITE_AUTH_RETURN_URL || "",
+ authBaseDomain: publicEnvVariables.VITE_AUTH_BASE_URL || "",
+ authReturnDomain:
+ publicEnvVariables.VITE_AUTH_RETURN_URL ??
+ (typeof window !== "undefined" ? window.location.origin : ""),
})}
Optional improvement: if VITE_AUTH_BASE_URL
is missing, render the control disabled with a tooltip instead of emitting a broken link.
Also applies to: 114-119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate definition and implementation of getPublicEnvVariables
rg -n "getPublicEnvVariables" -C3 .
# Locate definition and implementation of buildAuthLoginUrl
rg -n "buildAuthLoginUrl" -C3 .
Length of output: 35733
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Print the full implementation of buildAuthLoginUrl
sed -n '1,200p' apps/cyberstorm-remix/cyberstorm/utils/ThunderstoreAuth.tsx
Length of output: 504
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Display getPublicEnvVariables implementation
sed -n '1,200p' apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts
Length of output: 1547
Harden auth URL construction across codebase
- Replace
publicEnvVariables.VITE_AUTH_RETURN_URL || ""
with
publicEnvVariables.VITE_AUTH_RETURN_URL ?? window.location.origin
to guarantee an absolute return URL. - For
VITE_AUTH_BASE_URL
, either fallback towindow.location.origin
or render the link disabled (with tooltip) when missing. - Apply these changes in Connections.tsx (lines 75–78, 114–119) and every other
buildAuthLoginUrl
call in Navigation.tsx.
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/settings/user/Connections/Connections.tsx around
lines 75–78 and 114–119, replace uses of publicEnvVariables.VITE_AUTH_RETURN_URL
|| "" with publicEnvVariables.VITE_AUTH_RETURN_URL ?? window.location.origin so
the return URL is always absolute; for VITE_AUTH_BASE_URL, if missing fallback
to window.location.origin or render the auth link as disabled with a tooltip
explaining the missing base URL (and ensure the disabled state prevents
navigation and shows the tooltip); apply the same two changes to every
buildAuthLoginUrl call in Navigation.tsx (use the nullish coalescing fallback
for return URL and either fallback base URL to window.location.origin or render
the link disabled with tooltip when VITE_AUTH_BASE_URL is absent).
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
export function isPromise(value: any): value is Promise<unknown> { | ||
return typeof value?.then === "function"; | ||
} |
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.
Fix ESLint and widen the guard to PromiseLike.
Avoid any
and use a safer thenable check.
Apply:
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export function isPromise(value: any): value is Promise<unknown> {
- return typeof value?.then === "function";
-}
+export function isPromise(value: unknown): value is PromiseLike<unknown> {
+ return (
+ (typeof value === "object" || typeof value === "function") &&
+ value !== null &&
+ "then" in value &&
+ typeof (value as { then: unknown }).then === "function"
+ );
+}
📝 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.
// eslint-disable-next-line @typescript-eslint/no-explicit-any | |
export function isPromise(value: any): value is Promise<unknown> { | |
return typeof value?.then === "function"; | |
} | |
export function isPromise(value: unknown): value is PromiseLike<unknown> { | |
return ( | |
(typeof value === "object" || typeof value === "function") && | |
value !== null && | |
"then" in value && | |
typeof (value as { then: unknown }).then === "function" | |
); | |
} |
🧰 Tools
🪛 GitHub Check: ESLint
[failure] 5-5: Disallow the any
type
Unexpected any. Specify a different type.
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/cyberstorm/utils/typeChecks.ts around lines 4 to 7, the
isPromise function uses `any` and only narrows to Promise; change the signature
to accept `unknown` and narrow to `PromiseLike<unknown>` instead, remove the
eslint-disable, and implement a safe thenable check such as verifying the value
is not null/undefined and that (value as PromiseLike<unknown>).then is a
function (e.g., return value != null && typeof (value as
PromiseLike<unknown>).then === "function"); keep the type guard return type as
`value is PromiseLike<unknown>`.
&:hover { | ||
--tabs-item-color: var(--tab-nav-item-text-color--hover); | ||
--tabs-item-icon-color: var(--tab-nav-item-icon-color--hover); | ||
--tabs-item-border-bottom-color: var( | ||
--tab-nav-item-border-color--hover | ||
); | ||
} |
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.
🛠️ Refactor suggestion
Add focus-visible state mirroring hover for keyboard users
Parity for keyboard navigation avoids hidden focus.
Apply:
&:hover {
--tabs-item-color: var(--tab-nav-item-text-color--hover);
--tabs-item-icon-color: var(--tab-nav-item-icon-color--hover);
--tabs-item-border-bottom-color: var(
--tab-nav-item-border-color--hover
);
}
+ &:focus-visible {
+ --tabs-item-color: var(--tab-nav-item-text-color--hover);
+ --tabs-item-icon-color: var(--tab-nav-item-icon-color--hover);
+ --tabs-item-border-bottom-color: var(
+ --tab-nav-item-border-color--hover
+ );
+ }
And similarly for .tabs-number-slate:
&:hover {
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--hover);
--tabs-item-number-slate-background-color: var(
--tab-nav-item-border-color--hover
);
}
+ &:focus-visible {
+ --tabs-item-number-slate-color: var(--tab-nav-item-text-color--hover);
+ --tabs-item-number-slate-background-color: var(
+ --tab-nav-item-border-color--hover
+ );
+ }
Also applies to: 78-84
🤖 Prompt for AI Agents
In packages/cyberstorm-theme/src/components/Tabs/Tabs.css around lines 48-54
(and similarly for lines 78-84), the hover rule sets CSS custom properties for
colors/border but there is no keyboard-visible focus state; add a :focus-visible
rule that mirrors the &:hover block (setting --tabs-item-color,
--tabs-item-icon-color, and --tabs-item-border-bottom-color to the same hover
variables) so keyboard users receive the same visible styling; also apply the
same :focus-visible mirror for the .tabs-number-slate variant in both places.
&:active { | ||
--tabs-item-number-slate-color: var( | ||
--tab-nav-item-text-color--active | ||
); | ||
--tabs-item-number-slate-border-bottom-color: var( | ||
--tab-nav-item-border-color--active | ||
); | ||
} | ||
|
||
&:hover { | ||
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--hover); | ||
--tabs-item-number-slate-border-bottom-color: var( | ||
--tab-nav-item-border-color--hover | ||
); | ||
} | ||
&:hover { | ||
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--hover); | ||
--tabs-item-number-slate-border-bottom-color: var( | ||
--tab-nav-item-border-color--hover | ||
); | ||
} | ||
|
||
&.tabs__item--current { | ||
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--current); | ||
--tabs-item-number-slate-border-bottom-color: var( | ||
--tab-nav-item-border-color--current | ||
); | ||
&.tabs-item--current { | ||
--tabs-item-number-slate-color: var( | ||
--tab-nav-item-text-color--current | ||
); | ||
--tabs-item-number-slate-border-bottom-color: var( | ||
--tab-nav-item-border-color--current | ||
); | ||
} |
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.
Likely wrong variable: number-slate updates use “border-bottom-color” but base uses “background-color”
Active/hover/current update --tabs-item-number-slate-border-bottom-color, but the base and size blocks set --tabs-item-number-slate-background-color. If no consumer reads the “border-bottom-color” var, state styles won’t apply.
Apply this diff to align the state vars with the background var:
- --tabs-item-number-slate-border-bottom-color: var(
- --tab-nav-item-border-color--active
- );
+ --tabs-item-number-slate-background-color: var(
+ --tab-nav-item-border-color--active
+ );
@@
- --tabs-item-number-slate-border-bottom-color: var(
- --tab-nav-item-border-color--hover
- );
+ --tabs-item-number-slate-background-color: var(
+ --tab-nav-item-border-color--hover
+ );
@@
- --tabs-item-number-slate-border-bottom-color: var(
- --tab-nav-item-border-color--current
- );
+ --tabs-item-number-slate-background-color: var(
+ --tab-nav-item-border-color--current
+ );
If the intent was to style a border, confirm there’s a rule that consumes --tabs-item-number-slate-border-bottom-color; otherwise prefer the background var for visual parity with default state.
📝 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.
&:active { | |
--tabs-item-number-slate-color: var( | |
--tab-nav-item-text-color--active | |
); | |
--tabs-item-number-slate-border-bottom-color: var( | |
--tab-nav-item-border-color--active | |
); | |
} | |
&:hover { | |
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--hover); | |
--tabs-item-number-slate-border-bottom-color: var( | |
--tab-nav-item-border-color--hover | |
); | |
} | |
&:hover { | |
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--hover); | |
--tabs-item-number-slate-border-bottom-color: var( | |
--tab-nav-item-border-color--hover | |
); | |
} | |
&.tabs__item--current { | |
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--current); | |
--tabs-item-number-slate-border-bottom-color: var( | |
--tab-nav-item-border-color--current | |
); | |
&.tabs-item--current { | |
--tabs-item-number-slate-color: var( | |
--tab-nav-item-text-color--current | |
); | |
--tabs-item-number-slate-border-bottom-color: var( | |
--tab-nav-item-border-color--current | |
); | |
} | |
&:active { | |
--tabs-item-number-slate-color: var( | |
--tab-nav-item-text-color--active | |
); | |
--tabs-item-number-slate-background-color: var( | |
--tab-nav-item-border-color--active | |
); | |
} | |
&:hover { | |
--tabs-item-number-slate-color: var(--tab-nav-item-text-color--hover); | |
--tabs-item-number-slate-background-color: var( | |
--tab-nav-item-border-color--hover | |
); | |
} | |
&.tabs-item--current { | |
--tabs-item-number-slate-color: var( | |
--tab-nav-item-text-color--current | |
); | |
--tabs-item-number-slate-background-color: var( | |
--tab-nav-item-border-color--current | |
); | |
} |
🤖 Prompt for AI Agents
In packages/cyberstorm-theme/src/components/Tabs/Tabs.css around lines 69–92,
the active/hover/current states set --tabs-item-number-slate-border-bottom-color
while the base and size rules use --tabs-item-number-slate-background-color, so
state changes won’t affect the visible background; change the state declarations
to update --tabs-item-number-slate-background-color (unless the intent is to
style a border—in that case add/confirm a rule that consumes
--tabs-item-number-slate-border-bottom-color for the border and keep the state
vars as-is).
interface TabsProps extends Omit<FrameWindowProps, "primitiveType"> { | ||
csVariant?: TabsVariants; | ||
csSize?: TabsSizes; | ||
tabItems: { | ||
itemProps: IP; | ||
current: boolean; | ||
numberSlateValue?: number; | ||
disabled?: boolean; | ||
tabItemNumberSlateRootClasses?: string; | ||
key: string; | ||
}[]; | ||
renderTabItem: ( | ||
key: string, | ||
itemProps: IP, | ||
numberSlate?: ReactElement | ||
) => ReturnType<typeof NewLink>; | ||
ref?: React.Ref<HTMLDivElement>; | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Potential type mismatch: csSize default “medium” vs TabsSizes union
TabsSizes from theme may be "16" | "14" | "12" while Tabs defaults to "medium". If so, this won’t type-check and the generated class “tabs--size--medium” won’t match theme types.
Run:
If TabsSizes lacks "medium", either:
- Add "medium" to the union in the theme/types, or
- Change the default here to a valid token (e.g., "14") and align CSS selectors accordingly.
I can craft the follow-up diff once we confirm which token set you want to keep canonical.
Also applies to: 17-19
🏁 Script executed:
#!/bin/bash
rg -n 'export type TabsSizes' packages/cyberstorm-theme | sed -n '1,80p'
rg -n 'tabs--size--' packages -g '!**/dist/**' -C2
Length of output: 619
Fix csSize type mismatch
TabsSizes
in packages/cyberstorm-theme/src/components/Tabs/Tabs.ts
only allows "16" | "14" | "12"
, but CSS (Tabs.css
) and the csSize
default in packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx
use "medium"
. Either add "medium"
to TabsSizes
or change the default to one of the existing tokens (and update the CSS selectors accordingly).
🤖 Prompt for AI Agents
In packages/cyberstorm/src/newComponents/Tabs/Tabs.tsx lines 10-13, the csSize
prop type (TabsSizes) doesn't include the "medium" token used in CSS and as the
component default; update the TabsSizes union in
packages/cyberstorm-theme/src/components/Tabs/Tabs.ts to include "medium" (i.e.,
add "medium" to the allowed string literals), ensure the type is
exported/consumed by the Tabs component, and run the build/tests to verify no
other selectors/types break.
Fix sentry dsn client key not being in window env
Fix auth link building
Refactor Tabs to be more stable and simple
Fix package and wiki pages data loading on demand
Summary by CodeRabbit