Refactor many pages to support proper suspensions #1524
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx
Fixed
Show fixed
Hide fixed
apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx
Fixed
Show fixed
Hide fixed
apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx
Fixed
Show fixed
Hide fixed
WalkthroughThis PR restructures routes under community paths, overhauls community and package pages to Suspense/Await-driven async loaders, adds selective revalidation, replaces breadcrumbs rendering centrally in root, updates many CSS tokens (gaps, palettes, skeletons), renames Wiki/Versions CSS namespaces, and adjusts theme sizes/colors and component props. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router as Remix Router
participant Loader as Route loader
participant Dapper as API Client
participant UI as React (Suspense/Await)
User->>Router: Navigate to route
Router->>Loader: Invoke loader/clientLoader
Loader->>Dapper: Fetch data (often return Promise)
Loader-->>Router: { data: Promise<T> | T }
Router->>UI: Render with data
UI->>UI: Show Skeleton fallback (Suspense)
UI->>Router: Await resolve(data)
Router-->>UI: Resolved value
UI->>User: Render content
sequenceDiagram
autonumber
participant Nav as Navigation
participant SR as shouldRevalidate()
participant Default as defaultShouldRevalidate
Nav->>SR: currentUrl, nextUrl
alt same top-level segment
SR-->>Nav: false (skip revalidate)
else different
SR->>Default: delegate
Default-->>Nav: boolean
end
sequenceDiagram
autonumber
participant Root as root.tsx
participant Matches as useMatches()
participant Crumb as Breadcrumb builders
participant Await as Suspense/Await
Root->>Matches: Inspect route IDs
Root->>Crumb: Build crumbs (some Promises)
Crumb->>Await: Wrap async crumb pieces
Await-->>Root: Resolved icon/text
Root->>User: Render breadcrumbs
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
…_mobile_responsivity Update styles and mobile responsivity
…_and_some_improvements_to_perceived_performance Uh, more skeletons? And some improvements to perceived performance
…ading_and_improve_env_variable_logic Add image lazy loading and improve env variable logic
Oksamies
deleted the
09-07-refactor_many_pages_to_support_proper_suspensions
branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
apps/cyberstorm-remix/app/tools/manifest-validator/manifestValidator.tsx (2)
127-129: Encode as UTF‑8, not “binary/latin1”.Using "binary" can corrupt non-ASCII JSON. UTF‑8 is safer for user-entered manifest text.
-const encode = (str: string): string => - Buffer.from(str, "binary").toString("base64"); +const encode = (str: string): string => + Buffer.from(str, "utf-8").toString("base64");
152-189: Harden error handling and avoid throwing from an unawaited async.
- Provide a default failure message for API errors that don’t match known shapes.
- Don’t rethrow in the non-API branch; it will surface as an unhandled rejection since the promise isn’t awaited.
- Optional: accept AbortSignal to prevent setting state after abort.
-async function ManifestValidationResult( +async function ManifestValidationResult( teamInput: string, manifestInput: string, config: () => RequestConfig, - setValidation: (validation: { + setValidation: (validation: { status: "waiting" | "processing" | "success" | "failure"; message?: string; - }) => void + }) => void, + signal?: AbortSignal ) { try { const response = await toolsManifestValidate({ config: config, data: { namespace: teamInput, manifest_data: encode(manifestInput), }, params: {}, queryParams: {}, }); - if (response.success) { - setValidation({ status: "success", message: "All systems go!" }); - } + if (signal?.aborted) return; + if (response.success) { + setValidation({ status: "success", message: "All systems go!" }); + } else { + setValidation({ status: "failure", message: "Validation failed" }); + } } catch (e) { if (isApiError(e)) { if (isHTMLResponse(e.responseJson)) { - if (e.responseJson.non_field_errors) { + if (e.responseJson.non_field_errors) { setValidation({ status: "failure", message: e.responseJson.non_field_errors[0], }); } else if ( e.responseJson.namespace && e.responseJson.namespace[0] === "Object not found" ) { setValidation({ status: "failure", message: "Namespace not found", }); - } + } else { + setValidation({ status: "failure", message: "Validation failed" }); + } } } else { - // REMIX TODO: Add sentry logging here - setValidation({ status: "failure", message: "Unknown error" }); - throw e; + // TODO: Add sentry logging here + setValidation({ status: "failure", message: "Unknown error" }); } } }packages/cyberstorm/src/newComponents/TextInput/TextInput.tsx (1)
111-132: Clear button shows when value is undefined.The condition value !== "" is true for undefined, so the button can render spuriously. Check for nullish first.
- {clearValue && fProps.value !== "" ? ( + {clearValue && fProps.value != null && fProps.value !== "" ? (apps/cyberstorm-remix/app/tools/markdown-preview/markdownPreview.tsx (2)
63-66: Fix misleading copy: “Manifest Validator” on a Markdown preview page.The headings/descriptions don’t match the tool’s purpose.
Apply:
- <p className="markdown-preview__title">Manifest Validator</p> - <p className="markdown-preview__description"> - Select a team to validate a package - </p> + <p className="markdown-preview__title">Markdown input</p> + <p className="markdown-preview__description"> + Type or paste Markdown to preview + </p>
37-46: Prevent stale updates from out-of-order responses.Rapid edits can cause older responses to overwrite newer state. Guard with a request id or abort previous request.
Example (guard by request id):
// at top-level const reqIdRef = useRef(0); // in useEffect, before calling the API: const myId = ++reqIdRef.current; setValidation({ status: "processing" }); MarkdownValidationResult( debouncedMarkdownPreviewInput, outletContext.requestConfig, (next) => { if (reqIdRef.current === myId) setHTML(next); }, (v) => { if (reqIdRef.current === myId) setValidation(v); }, );If
toolsMarkdownPreviewsupports AbortController, prefer aborting the previous in-flight request.apps/cyberstorm-remix/app/upload/upload.tsx (2)
591-606: Fix state mutation when removing community categories.You’re mutating formInputs.community_categories before spreading it, which can cause subtle state bugs. Clone-then-omit instead.
Apply:
- } else { - if ( - formInputs.community_categories && - formInputs.community_categories[community] - ) { - const temp = formInputs.community_categories; - delete temp[community]; - updateFormFieldState({ - field: "community_categories", - value: { - ...temp, - }, - }); - } - } + } else { + const { [community]: _omit, ...rest } = + formInputs.community_categories ?? {}; + updateFormFieldState({ + field: "community_categories", + value: Object.keys(rest).length ? rest : undefined, + }); + }
158-166: Add error handling around upload.start().Unhandled rejections here will lead to a poor UX. Catch and toast, and clean up handle/isDone accordingly.
- await upload.start(); - setUsermedia(upload.handle); - setIsDone(true); + try { + await upload.start(); + setUsermedia(upload.handle); + setIsDone(true); + } catch (e: any) { + toast.addToast({ + csVariant: "danger", + children: `Upload failed: ${e?.message ?? "Unknown error"}`, + duration: 8000, + }); + setHandle(undefined); + setIsDone(false); + }apps/cyberstorm-remix/app/settings/teams/Teams.tsx (1)
164-173: Align SubmitorOutput type with the actual API used.You type against postTeamCreate but call teamCreate, which risks type drift.
- type SubmitorOutput = Awaited<ReturnType<typeof postTeamCreate>>; + type SubmitorOutput = Awaited<ReturnType<typeof teamCreate>>;Optionally remove the unused postTeamCreate import.
apps/cyberstorm-remix/app/tools/package-format-docs/packageFormatDocs.tsx (1)
287-289: Incorrect example for “installers” field.A URL isn’t a valid example; this should be an array/object per the docs. Please correct to avoid misleading users.
- { - value: <CodeBox key="code-5" value='"https://example.com/"' />, - sortValue: 0, - }, + { + value: <CodeBox key="code-6" value='[]' />, + sortValue: 0, + },If you have a canonical schema, we can add a realistic installer declaration instead.
apps/cyberstorm-remix/app/p/packageListing.tsx (1)
263-266: Fix stale Suspense data: missing dependencies in useMemo.These
Promise.allmemos won’t recompute on route param changes, causing stale content. Include dependencies.- const listingAndCommunityPromise = useMemo( - () => Promise.all([listing, community]), - [] - ); + const listingAndCommunityPromise = useMemo( + () => Promise.all([listing, community]), + [listing, community] + ); - const listingAndPermissionsPromise = useMemo( - () => Promise.all([listing, permissions]), - [] - ); + const listingAndPermissionsPromise = useMemo( + () => Promise.all([listing, permissions]), + [listing, permissions] + ); - const listingAndTeamPromise = useMemo(() => Promise.all([listing, team]), []); + const listingAndTeamPromise = useMemo( + () => Promise.all([listing, team]), + [listing, team] + );Also applies to: 268-271, 273-274
apps/cyberstorm-remix/app/commonComponents/CheckboxList/CheckboxList.css (1)
102-124: Add focus-visible to the interactive icons.Improves WCAG 2.4.7 compliance.
.checkbox-list__icon { display: flex; align-items: center; justify-content: center; width: var(--space-20); height: var(--space-20); --icon-inline-size: var(--space-20); background-color: transparent; transition: ease-out var(--animation-duration-xs); + outline: none; + &:focus-visible { + outline: 2px solid var(--color-focus-ring, var(--color-accent-blue-7)); + outline-offset: 2px; + } &.checkbox-list__exclude-button { --icon-inline-size: var(--space-16); width: var(--space-20); height: var(--space-20); &:hover { --icon-color: var(--color-accent-red-8); } + &:focus-visible { + outline: 2px solid var(--color-accent-red-8); + outline-offset: 2px; + } } }
♻️ Duplicate comments (1)
apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.tsx (1)
151-151: Unused variable 'filters' flagged by static analysis.The
filtersvariable is destructured but never directly used in the component body.
🧹 Nitpick comments (66)
apps/cyberstorm-remix/app/commonComponents/PackageSearch/components/PackageCount/PackageCount.css (1)
2-10: Confirm alignment intent; prefer pushing the block to the row end vs. right-aligning its children.With
flex-wrap: wrap,justify-content: flex-endright-aligns items on each wrapped line, which can look odd if the count text wraps. If the goal is to place the whole.package-countblock at the end of its parent flex row, consider usingmargin-inline-start: autoand droppingjustify-content. Also confirm RTL behavior is desired (in RTL, “end” flips)..package-count { - display: flex; - flex: 1; - flex-wrap: wrap; - justify-content: flex-end; + display: flex; /* keep if internal layout needs flex */ + flex: 1; + flex-wrap: wrap; + margin-inline-start: auto; /* push block to row end within parent flex */ + /* justify-content: flex-end; */ /* remove unless children must be end-aligned */ color: var(--color-text-tertiary); font-size: var(--font-size-body-md); word-break: break-all; }packages/cyberstorm/src/newComponents/SelectSearch/SelectSearch.css (1)
139-139: Token migration looks good; consider a graceful fallback.To avoid regressions if the new token isn’t present in some themes, add a fallback to the legacy token.
- gap: var(--gap-2xs); + gap: var(--gap-2xs, var(--gap-xxs));packages/cyberstorm/src/components/CodeInput/CodeInput.module.css (1)
4-4: Large vertical gap—tune for small screens.
--gap-7xlmay be excessive on mobile. Consider a responsive override..root { display: flex; flex-direction: column; - gap: var(--gap-7xl); + gap: var(--gap-7xl); } + +@media (width <= 475px) { + .root { gap: var(--gap-3xl); } +}apps/cyberstorm-remix/app/settings/teams/team/tabs/Settings/Settings.css (1)
7-12: Gap token rename is consistent; check mobile spacing.Looks aligned with the new token scale. If forms feel too airy on narrow viewports, add a mobile override to tighten gaps.
@layer nimbus-layout { .leave-team-form__body { display: flex; flex-direction: column; - gap: var(--gap-2xl); + gap: var(--gap-2xl); align-items: flex-start; align-self: stretch; } /* Disband team Form */ .disband-team-form__body { display: flex; flex-direction: column; - gap: var(--gap-2xl); + gap: var(--gap-2xl); align-items: flex-start; align-self: stretch; } + + @media (width <= 475px) { + .leave-team-form__body, + .disband-team-form__body { gap: var(--gap-xl); } + } }Also applies to: 24-30
apps/cyberstorm-remix/app/tools/manifest-validator/manifestValidator.css (1)
44-45: Mobile gap aligns with new scale; consider fluid spacing.If the layout needs smoother scaling between breakpoints, a clamp can help.
- gap: var(--gap-3xl); + gap: clamp(var(--gap-xl), 6vw, var(--gap-3xl));packages/cyberstorm/src/newComponents/Drawer/Drawer.css (1)
31-35: Reduced drawer height: re-check usability and a11y.Dropping max-height to 60% and increasing the fake backdrop to 40% may constrain long content and affect tap targets. Consider viewport-aware sizing and safe areas.
.drawer__fake-backdrop { width: 100%; - height: 40%; + height: 40%; background: transparent; } .drawer { display: none; flex: 1 0 0; @@ - max-height: 60%; + /* Use dynamic viewport and leave room for safe areas/toolbars */ + max-height: min(60dvh, calc(100dvh - var(--space-24))); + /* iOS scrolling inertia */ + overscroll-behavior: contain;Also applies to: 44-47
packages/cyberstorm/src/newComponents/Image/Image.tsx (1)
19-23: Add intrinsic size and decoding for CLS/perf; keep lazy default.Great addition. To reduce CLS and improve decode behavior, wire
intrinsicWidth/Heightanddecoding. Optionally exposefetchPriorityfor LCP images.interface ImageProps extends Omit<FrameWindowProps, "primitiveType"> { src: string | null | undefined; @@ - loading?: "eager" | "lazy"; + loading?: "eager" | "lazy"; + decoding?: "async" | "sync" | "auto"; + fetchPriority?: "high" | "low" | "auto"; @@ - loading = "lazy", + loading = "lazy", + decoding = "async", + fetchPriority = "auto", ...forwardedProps } = props; @@ - <img src={src} loading={loading} alt={alt} className="image__src" /> + <img + src={src} + loading={loading} + decoding={decoding} + fetchPriority={fetchPriority} + alt={alt} + className="image__src" + width={intrinsicWidth} + height={intrinsicHeight} + />If React version doesn’t support
fetchPriority, drop that prop.Also applies to: 39-41, 70-71
apps/cyberstorm-remix/app/tools/manifest-validator/manifestValidator.tsx (2)
76-109: Breadcrumb removal LGTM; consider de-duplicating the page title.You now render both a PageHeader "Manifest Validator" and a .manifest-validator__title with the same text. If the header is canonical, drop the inner title to reduce repetition.
<div className="manifest-validator__meta"> - <p className="manifest-validator__title">Manifest Validator</p> <p className="manifest-validator__description"> Select a team to validate a package </p> </div>
90-99: Differentiate “not logged in” vs “no teams”.Right now the alert only covers not logged in. Consider guiding users who are logged in but have zero teams.
{currentUser && currentUser.username ? ( currentUser.teams?.length ? null : ( <NewAlert csVariant="info">You don’t belong to any teams yet.</NewAlert> ) ) : ( <NewAlert csVariant="warning">You must be logged in to see your teams</NewAlert> )}apps/cyberstorm-remix/app/styles/layout.css (1)
6-9: Non-standard property: gate interpolate-size with @supports (optional).interpolate-size is still emerging; unknown properties are ignored, but wrapping in @supports reduces noise and future-proofs stylelint rules.
- /* stylelint-disable */ - interpolate-size: allow-keywords; - /* stylelint-enable */ + /* stylelint-disable-next-line */ + @supports (interpolate-size: allow-keywords) { + interpolate-size: allow-keywords; + }packages/cyberstorm/src/newComponents/TextInput/TextInput.tsx (2)
97-106: Size class “textarea” likely has no theme styles.Passing csSize === "textarea" into componentClasses yields .text-input--size--textarea, but the theme defines only default/small. This may produce unintended styling.
If you keep the “as” prop, keep csSize to "default" | "small" regardless of type.
50-56: Avoid firing enterHook on textarea Enter.Typical UX: Enter inserts a newline; firing enterHook can be surprising. Gate by element type or require meta/ctrl.
if ( fProps.value && enterHook && e.key === "Enter" && (e.currentTarget as HTMLElement).tagName !== "TEXTAREA" ) { enterHook(fProps.value); }apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts (1)
56-61: Fix typo and avoid client-only guidance in SSR error.Message has a typo and suggests a “hard refresh” even on server errors. Make it neutral.
- throw new Error( - "Enviroment variables did not load correctly, please hard refresh page" - ); + throw new Error( + "Environment variables did not load correctly." + );apps/cyberstorm-remix/app/p/tabs/Required/Required.css (1)
2-4: Skeleton height: prefer min-height to avoid layout jumps on small content.-.package-required__skeleton { - height: 500px; -} +.package-required__skeleton { + min-height: 500px; +}packages/cyberstorm/src/newComponents/SkeletonBox/SkeletonBox.css (2)
7-7: Add a safe fallback color for skeleton background.If
--color-skeleton-bg-coloris unset, the skeleton may render transparent. Provide a fallback to the previous hue.- background: var(--color-skeleton-bg-color); + background: var(--color-skeleton-bg-color, hsl(236deg 36% 36% / 0.24));
1-9: Respect prefers-reduced-motion.Disable pulsing for users who request reduced motion.
@layer components { .skeleton { display: flex; width: 100%; height: 100%; border-radius: var(--skeleton-radius, var(--radius-md)); background: var(--color-skeleton-bg-color); animation: pulse 2.5s infinite ease-in-out; } + @media (prefers-reduced-motion: reduce) { + .skeleton { + animation: none; + } + }Also applies to: 11-31
apps/cyberstorm-remix/app/p/tabs/Readme/Readme.tsx (3)
53-58: Align data shape withdefer(remove status/message branching).If
loader/clientLoaderthrow on error or usedefer,status/messagewon’t exist. Simplify the component to consume onlyreadmeand rely on Remix/route error boundaries and the<Await errorElement/>.-export default function Readme() { - const { status, message, readme } = useLoaderData< - typeof loader | typeof clientLoader - >(); - - if (status === "error") return <div>{message}</div>; +export default function Readme() { + const { readme } = useLoaderData<typeof loader>();
1-10: Minor: consolidate imports.You can import
useLoaderDataalongsideAwaitfrom the same module for consistency.-import { Await, LoaderFunctionArgs } from "react-router"; -import { useLoaderData } from "react-router"; +import { Await, LoaderFunctionArgs, useLoaderData } from "react-router";
32-41: GuardgetSessionTools()failure path.
getSessionTools()can throw when env vars are missing, which will crash the route. If that’s intended, consider throwing aResponsewith an appropriate status so the route-levelerrorElementhandles it gracefully.apps/cyberstorm-remix/app/p/tabs/Wiki/Wiki.css (1)
2-15: Set explicit min-widths for nav skeletons (optional).To avoid layout jumps with long labels, consider a min-width on
.package-wiki-navor the skeleton variant..package-wiki-nav__skeleton { height: 300px; + min-width: 12rem; }apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPageEdit.tsx (4)
237-260: Add proper ARIA roles for tabsFor better accessibility, prefer role="tablist"/"tab" with aria-selected instead of aria-current on buttons.
- <Tabs> + <Tabs role="tablist"> <button key="write" onClick={() => setSelectedTab("write")} - aria-current={selectedTab === "write"} + role="tab" + aria-selected={selectedTab === "write"} className={classnames( "package-wiki-edit__tabs-button", "tabs-item", selectedTab === "write" ? "tabs-item--current" : undefined )} > ... <button key="preview" onClick={() => setSelectedTab("preview")} - aria-current={selectedTab === "preview"} + role="tab" + aria-selected={selectedTab === "preview"} className={classnames( "package-wiki-edit__tabs-button", "tabs-item", selectedTab === "preview" ? "tabs-item--current" : undefined )} >
285-287: Move inline height to CSS class for consistencyInline style can be shifted to the namespaced class to centralize styling.
- rootClasses="package-wiki-edit__markdown-input" - style={{ height: "500px" }} + rootClasses="package-wiki-edit__markdown-input"Add to Wiki.css (or appropriate stylesheet):
.package-wiki-edit__markdown-input { min-height: 500px; }
299-319: Disable Save while submitting to avoid duplicate requestsIf useStrongForm exposes a submitting state, wire it to the button’s disabled prop.
- <NewButton - onClick={() => { - strongForm.submit(); - }} - csVariant="success" - > + <NewButton + onClick={() => { + strongForm.submit(); + }} + csVariant="success" + disabled={strongForm?.isSubmitting} + >If the API differs, swap to the correct flag (e.g., strongForm.state === "submitting"). Please confirm the hook’s API.
131-138: Fix user-facing toast messageThis path navigates to the wiki index, but the toast says “Moving to wiki page”. Update copy to avoid confusion.
- toast.addToast({ - csVariant: "info", - children: `Moving to wiki page`, - duration: 4000, - }); + toast.addToast({ + csVariant: "info", + children: `Moving to wiki`, + duration: 4000, + });apps/cyberstorm-remix/app/p/tabs/Wiki/WikiContent.tsx (1)
37-76: Class rename alignment + minor semantic HTML enhancementThe rename to package-wiki-content__* looks consistent. Consider wrapping the created date in a element for semantics.
- <span className="package-wiki-content__meta-date"> + <span className="package-wiki-content__meta-date"> <NewIcon csMode="inline" noWrapper> <FontAwesomeIcon icon={faCalendarDay} /> </NewIcon> - {page.datetime_created} + <time dateTime={page.datetime_created}> + {page.datetime_created} + </time> </span>apps/cyberstorm-remix/app/p/tabs/Wiki/WikiNewPage.tsx (3)
142-168: Apply tab roles for accessibilityMirror the ARIA enhancements suggested in the edit page for consistency.
- <Tabs> + <Tabs role="tablist"> <button key="write" onClick={() => setSelectedTab("write")} - aria-current={selectedTab === "write"} + role="tab" + aria-selected={selectedTab === "write"} className={classnames( "package-wiki-edit__tabs-button", "tabs-item", selectedTab === "write" ? "tabs-item--current" : undefined )} > ... <button key="preview" onClick={() => setSelectedTab("preview")} - aria-current={selectedTab === "preview"} + role="tab" + aria-selected={selectedTab === "preview"} className={classnames( "package-wiki-edit__tabs-button", "tabs-item", selectedTab === "preview" ? "tabs-item--current" : undefined )} >
191-193: Move inline height to CSSSame rationale as the edit page: centralize textarea height in CSS.
- rootClasses="package-wiki-edit__markdown-input" - style={{ height: "500px" }} + rootClasses="package-wiki-edit__markdown-input"Add to Wiki.css:
.package-wiki-edit__markdown-input { min-height: 500px; }
199-218: Footer class rename looks good; consider disabling Save while submittingSelectors updated correctly. If supported by useStrongForm, disable the Save button during submission to prevent duplicates.
apps/cyberstorm-remix/app/tools/package-format-docs/PackageFormatDocs.css (1)
3-3: Ensure gap applies (flex/grid container check)gap only affects flex/grid. Verify .package-format-docs is a flex/grid container where it’s used; if not, add display.
.package-format-docs { + display: flex; + flex-direction: column; gap: var(--gap-5xl); }apps/cyberstorm-remix/app/tools/markdown-preview/markdownPreview.tsx (2)
49-51: Align status message with page purpose.“Waiting for manifest text” is inconsistent here.
- message: "Waiting for manifest text", + message: "Waiting for Markdown input",
19-21: Consider moving the long placeholder HTML into a constant module.Keeps the component lean and reduces noise in diffs.
apps/cyberstorm-remix/app/upload/upload.tsx (1)
454-471: Prevent “Remove” from triggering the file picker.Stop event propagation and set the button type to avoid the DnD container click handler firing.
- <button + <button + type="button" className="drag-n-drop__remove-button" - onClick={() => { + onClick={(e) => { + e.preventDefault(); + e.stopPropagation(); // TODO: Clicking this causes the file select to open also setFile(null); if (fileInputRef.current) { fileInputRef.current.value = ""; } handle?.abort(); setHandle(undefined); setUsermedia(undefined); setIsDone(false); // setLock(false); }} >apps/cyberstorm-remix/app/upload/Upload.css (1)
260-266: Fix invalid CSS value for align-items.space-between isn’t valid for align-items; use center.
- align-items: space-between; + align-items: center;apps/cyberstorm-remix/app/p/tabs/Changelog/Changelog.css (1)
1-5: Prefer min-height over fixed height for skeleton.Avoid layout jumps on taller content.
- .package-changelog__skeleton { - height: 500px; - } + .package-changelog__skeleton { + min-height: 500px; + }apps/cyberstorm-remix/app/communities/communities.tsx (1)
82-87: Simplify null checks when calling getCommunities.order is already defaulted; pass it directly and tighten search handling.
- communities: await dapper.getCommunities( - page, - order === null ? undefined : order, - search === null ? undefined : search - ), + communities: await dapper.getCommunities( + page, + order, + search ?? undefined + ),packages/cyberstorm/src/newComponents/BreadCrumbs/BreadCrumbs.css (1)
12-24: Avoid flex on img; tighten inline content layout.display:flex on img is unnecessary and can cause layout quirks. Prefer inline-block and drop centering rules.
- img { - display: flex; - align-items: center; - justify-content: center; - width: 1rem; - height: 1rem; - aspect-ratio: 1/1; - } + img { + display: inline-block; + width: 1rem; + height: 1rem; + aspect-ratio: 1/1; + }The span > span inline-flex rules look good for icon+label spacing.
Also applies to: 26-41
apps/cyberstorm-remix/app/commonComponents/CheckboxList/CheckboxList.tsx (1)
25-26: Consider removing unused helper function.The
nextStateResolvefunction is no longer used after the refactoring and can be safely removed.-const nextStateResolve = (state: TRISTATE | boolean) => - typeof state === "string" ? resolveTriState(state) : !state; -apps/cyberstorm-remix/app/p/packageListing.tsx (3)
417-429: Prefer skeleton over text fallback in Drawer meta.For consistency with the rest of the page, replace
<p>Loading...</p>with a small skeleton variant.- <Suspense fallback={<p>Loading...</p>}> + <Suspense + fallback={<SkeletonBox className="package-listing-sidebar__skeleton" />} + >
430-443: Same as above: unify Drawer boxes fallback with skeleton.Aligns loading states across sections.
- <Suspense fallback={<p>Loading...</p>}> + <Suspense + fallback={<SkeletonBox className="package-listing-sidebar__boxes-skeleton" />} + >
444-457: Unify Actions fallback to skeleton for narrow layout.Use the same actions skeleton used in the sidebar.
- <Suspense fallback={<p>Loading...</p>}> + <Suspense + fallback={<SkeletonBox className="package-listing-sidebar__actions-skeleton" />} + >apps/cyberstorm-remix/app/p/tabs/Wiki/WikiPage.tsx (1)
53-76: Try/catch no longer handles API errors.Since
getPackageWiki/getPackageWikiPageare not awaited, thetry/catchwon’t intercept rejected promises (404, 500). Errors now surface in<Await>. Either remove the try/catch or convert toawaitif you still want loader-time branching for 404.
- Option A (preferred with Suspense): Remove try/catch; let
<Await errorElement>or a route error boundary handle.- Option B: Reintroduce
awaitand preserve 404 → “not found” behavior directly in the loader.Also applies to: 119-149
apps/cyberstorm-remix/app/p/tabs/Required/Required.tsx (1)
59-94: Use a stable key for dependency items.Avoid array index as a key to prevent suboptimal reconciliation on list updates.
- {resolvedValue.dependencies.map((dep, key) => { - return ( - <ListingDependency - key={key} + {resolvedValue.dependencies.map((dep) => { + return ( + <ListingDependency + key={`${dep.namespace}/${dep.name}@${dep.version_number}`} dependency={dep} // TODO: Remove when package versiond detail is available domain={outletContext.domain} /> ); })}apps/cyberstorm-remix/app/p/packageListing.css (2)
15-16: Remove duplicate align-items declaration.Line 16 overrides Line 15. Keep one to avoid confusion.
- align-items: center; - align-items: flex-start; + align-items: flex-start;
109-118: Prefer overflow-x: auto for content section.Prevents always-on scrollbars and honors native platform behavior.
- overflow-x: scroll; + overflow-x: auto;apps/cyberstorm-remix/app/p/tabs/Versions/Versions.css (1)
13-16: Use overflow-x: auto instead of scroll.Same rationale: avoids forced scrollbars.
- overflow-x: scroll; + overflow-x: auto;apps/cyberstorm-remix/app/commonComponents/PackageSearch/PackageSearch.css (1)
130-139: Grid skeleton height seems high on wide screens.Optional: reduce slightly (e.g., 26–28rem) to limit scroll on sparse results.
apps/cyberstorm-remix/app/communities/Communities.css (1)
52-53: Good move to centralize skeleton rounding via --skeleton-radius.Consider applying the same var to title/meta for full consistency.
apps/cyberstorm-remix/app/commonComponents/CheckboxList/CheckboxList.css (1)
1-127: Rename remaining identifiers: checkbox → iconNo .checkbox-list__checkbox class remains, but TSX still uses htmlFor/id
checkbox-list__checkbox__${item.label}— update those tocheckbox-list__icon__${item.label}for consistency.
Location: apps/cyberstorm-remix/app/commonComponents/CheckboxList/CheckboxList.tsx (htmlFor / id occurrences).apps/cyberstorm-remix/app/c/tabs/PackageSearch/PackageSearch.tsx (3)
31-49: Parallelize network calls in the server loader.Fetch filters and listings concurrently.
- const filters = await dapper.getCommunityFilters(params.communityId); - - return { - filters: filters, - listings: await dapper.getPackageListings( + const [filters, listings] = await Promise.all([ + dapper.getCommunityFilters(params.communityId), + dapper.getPackageListings( { kind: "community", communityId: params.communityId, }, - ordering ?? "", - page === null ? undefined : Number(page), - search ?? "", - includedCategories?.split(",") ?? undefined, - excludedCategories?.split(",") ?? undefined, - section ? (section === "all" ? "" : section) : "", - nsfw === "true" ? true : false, - deprecated === "true" ? true : false - ), - }; + ordering, + page, + search, + includedCategories, + excludedCategories, + section, + nsfw, + deprecated + ), + ]); + return { filters, listings };
60-65: Avoid repeated getConfig() calls.Slight cleanup; cache the result.
- const dapper = new DapperTs(() => { - return { - apiHost: tools?.getConfig().apiHost, - sessionId: tools?.getConfig().sessionId, - }; - }); + const cfg = tools.getConfig(); + const dapper = new DapperTs(() => ({ + apiHost: cfg.apiHost, + sessionId: cfg.sessionId, + }));
66-76: Deduplicate param parsing between loader/clientLoader.Extract a shared parse function to keep logic in sync.
Also applies to: 84-92
apps/cyberstorm-remix/app/c/community.tsx (1)
141-147: Hero image: hint to prioritize above-the-foldIf this hero is above the fold, consider fetchPriority to improve LCP.
- <img + <img + fetchPriority="high" src={resolvedValue.hero_image_url} alt={resolvedValue.name} />apps/cyberstorm-remix/app/p/dependants/Dependants.tsx (3)
20-39: Query param parsing duplicated across routesThis block mirrors PackageSearch and others. Consider a shared helper for ordering/page/search/category/nsfw/deprecated to keep behavior uniform.
41-66: Remove unused ‘community’ from loader payloadYou don’t consume community in this route. Trim to reduce work and payload.
- return { - community: dapper.getCommunity(params.communityId), + return { listing: dapper.getPackageListingDetails( params.communityId, params.namespaceId, params.packageId ),Apply the same in clientLoader.
67-68: Error message specificityThis route requires communityId, namespaceId, and packageId; “Package not found” (or “Resource not found”) is more accurate.
- throw new Response("Community not found", { status: 404 }); + throw new Response("Package not found", { status: 404 });Also applies to: 119-120
apps/cyberstorm-remix/app/p/tabs/Versions/Versions.tsx (1)
109-145: Semver sort direction sanity-checkCustom comparator respects columnMeta.direction, but initial sort is ASC. If you want newest first by default, set DESC.
- sortDirection={NewTableSort.ASC} + sortDirection={NewTableSort.DESC}packages/cyberstorm-theme/src/components/componentsColors.css (1)
181-188: Normalize transparency literals for consistency.Mixing
#0000andhsl(... / 0)can be confusing. Pick one (e.g.,hsl(0deg 0% 0% / 0)ortransparent) and standardize.Also applies to: 311-325
apps/cyberstorm-remix/app/p/tabs/Changelog/Changelog.tsx (1)
63-71: Fallback UX is good; keep generic error copy consistent site-wide.The Suspense fallback and
errorElementare fine. Consider centralizing the error message to a shared i18n key later.apps/cyberstorm-remix/app/root.tsx (3)
133-139: Fix typo in user-facing error message.“Enviroment” → “Environment”.
- "Enviroment variables did not load correctly, please hard refresh page" + "Environment variables did not load correctly, please hard refresh the page"
199-207: Reduce false-positive updates of public env vars.Strict reference inequality (
!==) will almost always be true for objects. Use a shallow/deep compare to avoid rewritingwindow.NIMBUS_PUBLIC_ENVunnecessarily.- if ( - data && - data.publicEnvVariables && - data.publicEnvVariables !== envVars - ) { + if (data?.publicEnvVariables && JSON.stringify(data.publicEnvVariables) !== JSON.stringify(envVars)) { shouldUpdatePublicEnvVars = true; }
552-611: Breadcrumb: good use of Suspense/Await; minor perf nit.
isRecordchecks are fine. You can memoize the derived{label, icon}by keying oncommunityPage.params.communityIdto avoid re-renders on unrelated state changes.apps/cyberstorm-remix/app/p/team/Team.tsx (1)
105-107: Consider type safety for useLoaderData return type.Using a union type
typeof loader | typeof clientLoadermight not provide accurate typing if the two loaders return different shapes (e.g., one returns promises, the other doesn't).Consider defining a shared type for both loaders to ensure consistency:
type TeamLoaderData = { teamId: string; filters: CommunityFilters | Promise<CommunityFilters>; listings: PackageListings | Promise<PackageListings>; }; // Then use: const { filters, listings, teamId } = useLoaderData<TeamLoaderData>();apps/cyberstorm-remix/app/routes.ts (1)
38-43: Consider nestingTeamunder the community route to share layout/crumbs and loaders.Keeping
Teamoutside the/c/:communityIdchildren forfeits parent layout, Suspense boundaries, and breadcrumb composition. Nest it under the existingprefix("p", ...).- route("/c/:communityId/p/:namespaceId", "p/team/Team.tsx"), + // Move this into the children of route("/c/:communityId", "c/community.tsx", ...): + // ...prefix("p", [ + // route(":namespaceId", "p/team/Team.tsx"), + // ... + // ])If nesting is intentional, verify breadcrumbs and community header still render on the Team page. If they don’t, nesting will fix it.
apps/cyberstorm-remix/app/c/Community.css (3)
23-43: Avoid animatingheight; prefermax-height/opacity and disable on reduced motion.
transition: height 2sis layout-intensive and janky on low-end devices. Also preferoverflow: hiddento cover both axes.- overflow-y: hidden; - transition: height 2s; + overflow: hidden; + transition: height 400ms ease;Additionally, add a reduced-motion override elsewhere in this file:
@media (prefers-reduced-motion: reduce) { .community__background, .community__content-header-wrapper, .community__content-header--hide { transition: none; } }
91-102: Normalize token casing to match theme variables; keep fallbacks.Mixed-case tokens likely don’t resolve given the palette migration; fallbacks mask the issue and break theming.
- padding: var(--Space-12, 0.75rem); - border: 1px solid var(--Color-border-bright, rgb(70 70 149 / 0.66)); - border-radius: var(--Radius-xl, 1rem); - background: var(--Color-Surface-1, #070721); + padding: var(--space-12, 0.75rem); + border: 1px solid var(--color-border-bright, rgb(70 70 149 / 0.66)); + border-radius: var(--radius-xl, 1rem); + background: var(--color-ui-surface-1, #070721);
34-38: Minor: duplicate surface token usage—prefer one canonical token.
background: var(--color-ui-surface-1);already exists above; ensure all surfaces reference the same token to avoid subtle mismatches.
| .community__content-header--hide { | ||
| height: 0; | ||
| visibility: collapse; | ||
| opacity: 0; | ||
| transition: | ||
| height ease 1s, | ||
| opacity 0.8s 0.2s, | ||
| visibility 1s 0s; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
visibility: collapse isn’t widely supported outside table/grid. Use hidden and disable interaction.
This makes the hide state reliable across browsers.
- visibility: collapse;
+ visibility: hidden;
+ pointer-events: none;📝 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.
| .community__content-header--hide { | |
| height: 0; | |
| visibility: collapse; | |
| opacity: 0; | |
| transition: | |
| height ease 1s, | |
| opacity 0.8s 0.2s, | |
| visibility 1s 0s; | |
| } | |
| .community__content-header--hide { | |
| height: 0; | |
| visibility: hidden; | |
| pointer-events: none; | |
| opacity: 0; | |
| transition: | |
| height ease 1s, | |
| opacity 0.8s 0.2s, | |
| visibility 1s 0s; | |
| } |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/c/Community.css around lines 81 to 89, the
.community__content-header--hide rule uses visibility: collapse which isn’t
reliably supported outside table/grid contexts; change it to visibility: hidden
and also disable interaction by adding pointer-events: none (optionally keep
overflow: hidden for safety) so the hidden state works consistently across
browsers while preserving the existing height/opacity/transition behavior.
| export function shouldRevalidate(arg: ShouldRevalidateFunctionArgs) { | ||
| if ( | ||
| arg.currentUrl.pathname.split("/")[1] === arg.nextUrl.pathname.split("/")[1] | ||
| ) { | ||
| return false; | ||
| } | ||
| return arg.defaultShouldRevalidate; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Over-broad shouldRevalidate blocks cross-community navigations
Comparing only segment [1] (“c”) prevents revalidation when switching between different communities (e.g., /c/a → /c/b). Compare the community slug (segment [2]) instead.
Apply:
export function shouldRevalidate(arg: ShouldRevalidateFunctionArgs) {
- if (
- arg.currentUrl.pathname.split("/")[1] === arg.nextUrl.pathname.split("/")[1]
- ) {
- return false;
- }
- return arg.defaultShouldRevalidate;
+ const oldPath = arg.currentUrl.pathname.split("/");
+ const newPath = arg.nextUrl.pathname.split("/");
+ // Don't revalidate when staying within the same community
+ if (oldPath[1] === "c" && newPath[1] === "c" && oldPath[2] === newPath[2]) {
+ return false;
+ }
+ return arg.defaultShouldRevalidate;
}📝 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.
| export function shouldRevalidate(arg: ShouldRevalidateFunctionArgs) { | |
| if ( | |
| arg.currentUrl.pathname.split("/")[1] === arg.nextUrl.pathname.split("/")[1] | |
| ) { | |
| return false; | |
| } | |
| return arg.defaultShouldRevalidate; | |
| } | |
| export function shouldRevalidate(arg: ShouldRevalidateFunctionArgs) { | |
| const oldPath = arg.currentUrl.pathname.split("/"); | |
| const newPath = arg.nextUrl.pathname.split("/"); | |
| // Don't revalidate when staying within the same community | |
| if (oldPath[1] === "c" && newPath[1] === "c" && oldPath[2] === newPath[2]) { | |
| return false; | |
| } | |
| return arg.defaultShouldRevalidate; | |
| } |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/c/community.tsx around lines 67 to 74, the current
shouldRevalidate logic only compares pathname segment [1] (which is "c") and
thus prevents revalidation when switching between communities; update the
comparison to use segment [2] (the community slug) instead, e.g. extract
segments safely (checking length) and compare segments[2] of currentUrl and
nextUrl, returning false only when they match and otherwise falling back to
arg.defaultShouldRevalidate.
| title={`Thunderstore - The ${resolvedValue.name} Mod Database`} | ||
| /> | ||
| <meta | ||
| name="description" | ||
| content={`Mods for ${resolvedValue.name}`} | ||
| /> | ||
| <meta property="og:type" content="website" /> | ||
| <meta | ||
| property="og:url" | ||
| content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"])}${ | ||
| location.pathname | ||
| }`} | ||
| /> | ||
| <meta | ||
| property="og:title" | ||
| content={`Thunderstore - The ${resolvedValue.name} Mod Database`} | ||
| /> | ||
| <meta | ||
| property="og:description" | ||
| content={`Thunderstore is a mod database and API for downloading ${resolvedValue.name} mods`} | ||
| /> | ||
| <meta property="og:image:width" content="360" /> | ||
| <meta property="og:image:height" content="480" /> | ||
| <meta | ||
| property="og:image" | ||
| content={resolvedValue.cover_image_url ?? undefined} | ||
| /> | ||
| <meta property="og:site_name" content="Thunderstore" /> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Meta tags rendered in body (ineffective) + invalid + broken og:url
- isn’t valid; use <title> or a route meta export.
- Putting in the body won’t populate the document head.
- og:url builds from an object, yielding “[object Object]/path”.
Remove the inline meta block:
- <meta
- title={`Thunderstore - The ${resolvedValue.name} Mod Database`}
- />
- <meta
- name="description"
- content={`Mods for ${resolvedValue.name}`}
- />
- <meta property="og:type" content="website" />
- <meta
- property="og:url"
- content={`${getPublicEnvVariables(["VITE_BETA_SITE_URL"])}${
- location.pathname
- }`}
- />
- <meta
- property="og:title"
- content={`Thunderstore - The ${resolvedValue.name} Mod Database`}
- />
- <meta
- property="og:description"
- content={`Thunderstore is a mod database and API for downloading ${resolvedValue.name} mods`}
- />
- <meta property="og:image:width" content="360" />
- <meta property="og:image:height" content="480" />
- <meta
- property="og:image"
- content={resolvedValue.cover_image_url ?? undefined}
- />
- <meta property="og:site_name" content="Thunderstore" />Add a proper route meta export (outside this block):
// at top-level of this module
import type { MetaFunction } from "@remix-run/node";
export const meta: MetaFunction<typeof loader> = ({ data, location }) => {
const base =
getPublicEnvVariables(["VITE_BETA_SITE_URL"]).VITE_BETA_SITE_URL ?? "";
const pathname = location?.pathname ?? "";
const c = data?.community as
| { name?: string; cover_image_url?: string }
| undefined;
const name = c?.name ?? "Thunderstore";
const url = `${base}${pathname}`;
const tags = [
{ title: `Thunderstore - The ${name} Mod Database` },
{ name: "description", content: `Mods for ${name}` },
{ property: "og:type", content: "website" },
{ property: "og:url", content: url },
{ property: "og:title", content: `Thunderstore - The ${name} Mod Database` },
{
property: "og:description",
content: `Thunderstore is a mod database and API for downloading ${name} mods`,
},
{ property: "og:image:width", content: "360" },
{ property: "og:image:height", content: "480" },
{ property: "og:site_name", content: "Thunderstore" },
] as const;
return c?.cover_image_url
? [...tags, { property: "og:image", content: c.cover_image_url }]
: [...tags];
};🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/c/community.tsx around lines 93–122, the review
points out invalid and ineffective inline meta tags: a nonstandard <meta title>
is used, <meta> elements are being rendered inside the body (so they won’t
populate the document head), and og:url concatenates an object causing “[object
Object]/path”. Remove the inline meta block from the JSX and instead add a
top-level Remix route meta export (MetaFunction) that builds meta tags
programmatically: extract the VITE_BETA_SITE_URL string from
getPublicEnvVariables, use location.pathname safely, compute name and
cover_image_url from loader data, return the proper tags array (including title
via { title: ... } and og:image only if cover_image_url exists) so the tags
render correctly in the document head.
| const searchParams = new URL(request.url).searchParams; | ||
| const ordering = | ||
| searchParams.get("ordering") ?? PackageOrderOptions.Updated; | ||
| const page = searchParams.get("page"); | ||
| const search = searchParams.get("search"); | ||
| const includedCategories = searchParams.get("includedCategories"); | ||
| const excludedCategories = searchParams.get("excludedCategories"); | ||
| const section = searchParams.get("section"); | ||
| const nsfw = searchParams.get("nsfw"); | ||
| const deprecated = searchParams.get("deprecated"); | ||
| const filters = await dapper.getCommunityFilters(params.communityId); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden query param parsing (ordering, page, categories).
Avoid NaN/0 pages, empty category tokens, and invalid orderings.
- const searchParams = new URL(request.url).searchParams;
- const ordering =
- searchParams.get("ordering") ?? PackageOrderOptions.Updated;
- const page = searchParams.get("page");
- const search = searchParams.get("search");
- const includedCategories = searchParams.get("includedCategories");
- const excludedCategories = searchParams.get("excludedCategories");
- const section = searchParams.get("section");
- const nsfw = searchParams.get("nsfw");
- const deprecated = searchParams.get("deprecated");
+ const sp = new URL(request.url).searchParams;
+ const orderingRaw = sp.get("ordering") ?? "";
+ const validOrderings = Object.values(PackageOrderOptions) as string[];
+ const ordering = validOrderings.includes(orderingRaw)
+ ? orderingRaw
+ : PackageOrderOptions.Updated;
+ const pageRaw = sp.get("page");
+ const page = pageRaw && /^\d+$/.test(pageRaw) ? Math.max(1, Number(pageRaw)) : undefined;
+ const search = sp.get("search") ?? "";
+ const incArr = (sp.get("includedCategories") || "")
+ .split(",")
+ .map((s) => s.trim())
+ .filter(Boolean);
+ const excArr = (sp.get("excludedCategories") || "")
+ .split(",")
+ .map((s) => s.trim())
+ .filter(Boolean);
+ const includedCategories = incArr.length ? incArr : undefined;
+ const excludedCategories = excArr.length ? excArr : undefined;
+ const sectionRaw = sp.get("section");
+ const section = sectionRaw && sectionRaw !== "all" ? sectionRaw : "";
+ const nsfw = sp.get("nsfw") === "true";
+ const deprecated = sp.get("deprecated") === "true";📝 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.
| const searchParams = new URL(request.url).searchParams; | |
| const ordering = | |
| searchParams.get("ordering") ?? PackageOrderOptions.Updated; | |
| const page = searchParams.get("page"); | |
| const search = searchParams.get("search"); | |
| const includedCategories = searchParams.get("includedCategories"); | |
| const excludedCategories = searchParams.get("excludedCategories"); | |
| const section = searchParams.get("section"); | |
| const nsfw = searchParams.get("nsfw"); | |
| const deprecated = searchParams.get("deprecated"); | |
| const filters = await dapper.getCommunityFilters(params.communityId); | |
| const sp = new URL(request.url).searchParams; | |
| const orderingRaw = sp.get("ordering") ?? ""; | |
| const validOrderings = Object.values(PackageOrderOptions) as string[]; | |
| const ordering = validOrderings.includes(orderingRaw) | |
| ? orderingRaw | |
| : PackageOrderOptions.Updated; | |
| const pageRaw = sp.get("page"); | |
| const page = pageRaw && /^\d+$/.test(pageRaw) ? Math.max(1, Number(pageRaw)) : undefined; | |
| const search = sp.get("search") ?? ""; | |
| const incArr = (sp.get("includedCategories") || "") | |
| .split(",") | |
| .map((s) => s.trim()) | |
| .filter(Boolean); | |
| const excArr = (sp.get("excludedCategories") || "") | |
| .split(",") | |
| .map((s) => s.trim()) | |
| .filter(Boolean); | |
| const includedCategories = incArr.length ? incArr : undefined; | |
| const excludedCategories = excArr.length ? excArr : undefined; | |
| const sectionRaw = sp.get("section"); | |
| const section = sectionRaw && sectionRaw !== "all" ? sectionRaw : ""; | |
| const nsfw = sp.get("nsfw") === "true"; | |
| const deprecated = sp.get("deprecated") === "true"; | |
| const filters = await dapper.getCommunityFilters(params.communityId); |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/c/tabs/PackageSearch/PackageSearch.tsx around lines
21-31, validate and sanitize the query params: parse ordering and fallback to
PackageOrderOptions.Updated if the value is not one of the enum values; parse
page with parseInt and if result is NaN or <1 default to 1; split
includedCategories and excludedCategories on commas, trim and filter out empty
tokens to avoid passing empty strings; coerce nsfw and deprecated to booleans
only when the string equals "true" (or use a safe helper); ensure section and
search are left as strings but normalized (trim) before use. Apply these checks
where the params are read so downstream code receives only valid ordering, a
positive page number, and non-empty category arrays.
| &.checkbox-list__label--off { | ||
| color: var(--color-text-secondary); | ||
|
|
||
| svg { | ||
| --icon-color: var(--color-surface-a8); | ||
| } | ||
|
|
||
| .checkbox-list__exclude-button svg { | ||
| --icon-color: transparent; | ||
| } | ||
| } | ||
|
|
||
| &.checkbox-list__label--include { | ||
| color: var(--color-cyber-green-7); | ||
| font-weight: var(--font-weight-bold); | ||
|
|
||
| svg { | ||
| .checkbox-list__checkbox-button svg { | ||
| --icon-color: var(--color-cyber-green-7); | ||
| } | ||
|
|
||
| .checkbox-list__exclude-button svg { | ||
| --icon-color: transparent; | ||
| } | ||
| } | ||
|
|
||
| &.checkbox-list__label--exclude { | ||
| color: var(--color-accent-red-7); | ||
| font-weight: var(--font-weight-bold); | ||
|
|
||
| svg { | ||
| .checkbox-list__exclude-button svg { | ||
| --icon-color: var(--color-accent-red-7); | ||
| } | ||
|
|
||
| .checkbox-list__checkbox-button svg { | ||
| --icon-color: var(--color-surface-a8); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
State-specific icon coloring is clear; add focus-visible styles for a11y.
Keyboard users need a visible focus ring on the icon buttons.
.checkbox-list__label {
+ &:focus-within {
+ outline: 2px solid var(--color-focus-ring, var(--color-accent-blue-7));
+ outline-offset: 2px;
+ }📝 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.
| &.checkbox-list__label--off { | |
| color: var(--color-text-secondary); | |
| svg { | |
| --icon-color: var(--color-surface-a8); | |
| } | |
| .checkbox-list__exclude-button svg { | |
| --icon-color: transparent; | |
| } | |
| } | |
| &.checkbox-list__label--include { | |
| color: var(--color-cyber-green-7); | |
| font-weight: var(--font-weight-bold); | |
| svg { | |
| .checkbox-list__checkbox-button svg { | |
| --icon-color: var(--color-cyber-green-7); | |
| } | |
| .checkbox-list__exclude-button svg { | |
| --icon-color: transparent; | |
| } | |
| } | |
| &.checkbox-list__label--exclude { | |
| color: var(--color-accent-red-7); | |
| font-weight: var(--font-weight-bold); | |
| svg { | |
| .checkbox-list__exclude-button svg { | |
| --icon-color: var(--color-accent-red-7); | |
| } | |
| .checkbox-list__checkbox-button svg { | |
| --icon-color: var(--color-surface-a8); | |
| } | |
| } | |
| .checkbox-list__label { | |
| &:focus-within { | |
| outline: 2px solid var(--color-focus-ring, var(--color-accent-blue-7)); | |
| outline-offset: 2px; | |
| } | |
| &.checkbox-list__label--off { | |
| color: var(--color-text-secondary); | |
| svg { | |
| --icon-color: var(--color-surface-a8); | |
| } | |
| .checkbox-list__exclude-button svg { | |
| --icon-color: transparent; | |
| } | |
| } | |
| &.checkbox-list__label--include { | |
| color: var(--color-cyber-green-7); | |
| font-weight: var(--font-weight-bold); | |
| .checkbox-list__checkbox-button svg { | |
| --icon-color: var(--color-cyber-green-7); | |
| } | |
| .checkbox-list__exclude-button svg { | |
| --icon-color: transparent; | |
| } | |
| } | |
| &.checkbox-list__label--exclude { | |
| color: var(--color-accent-red-7); | |
| font-weight: var(--font-weight-bold); | |
| .checkbox-list__exclude-button svg { | |
| --icon-color: var(--color-accent-red-7); | |
| } | |
| .checkbox-list__checkbox-button svg { | |
| --icon-color: var(--color-surface-a8); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/commonComponents/CheckboxList/CheckboxList.css
around lines 26 to 62, add focus-visible styles for the icon buttons so keyboard
users get a visible focus ring: for each state block
(&.checkbox-list__label--off, &.checkbox-list__label--include,
&.checkbox-list__label--exclude) add selectors targeting
.checkbox-list__checkbox-button:focus-visible and
.checkbox-list__exclude-button:focus-visible and apply an accessible focus
treatment (visible outline or focused box-shadow with sufficient contrast and
preserved icon color) and ensure the outline does not rely on mouse-only :focus
styles; keep styles consistent across states and avoid altering layout.
| route("/:slug", "p/tabs/Wiki/WikiPage.tsx"), | ||
| route("/:slug/edit", "p/tabs/Wiki/WikiPageEdit.tsx"), | ||
| route("/c/:communityId", "c/community.tsx", [ | ||
| route("/c/:communityId/", "c/tabs/PackageSearch/PackageSearch.tsx"), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use index() for the community default child instead of an absolute path.
Absolute paths inside nested children create separate, root-anchored routes and can cause duplicate matches. Make this the index route of /c/:communityId.
- route("/c/:communityId/", "c/tabs/PackageSearch/PackageSearch.tsx"),
+ index("c/tabs/PackageSearch/PackageSearch.tsx"),📝 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.
| route("/c/:communityId/", "c/tabs/PackageSearch/PackageSearch.tsx"), | |
| index("c/tabs/PackageSearch/PackageSearch.tsx"), |
🤖 Prompt for AI Agents
In apps/cyberstorm-remix/app/routes.ts around line 16, the child route is
registered with an absolute path which creates a separate root-anchored route
and can duplicate matches; replace the absolute route call with an index() route
for the community default child so the PackageSearch component becomes the index
child of /c/:communityId — i.e., change the route(...) invocation to use
index(...) (no leading slash) within the /c/:communityId children so it is
matched as the default nested route.
| route( | ||
| "/c/:communityId/p/:namespaceId/:packageId/", | ||
| "p/tabs/Readme/Readme.tsx" | ||
| ), |
There was a problem hiding this comment.
Make the Readme tab the nested index route for the package listing.
Using an absolute path here breaks nesting and layout composition. Use an index child under :namespaceId/:packageId.
- route(
- "/c/:communityId/p/:namespaceId/:packageId/",
- "p/tabs/Readme/Readme.tsx"
- ),
+ index("p/tabs/Readme/Readme.tsx"),📝 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.
| route( | |
| "/c/:communityId/p/:namespaceId/:packageId/", | |
| "p/tabs/Readme/Readme.tsx" | |
| ), | |
| index("p/tabs/Readme/Readme.tsx"), |
| layout("p/tabs/Wiki/Wiki.tsx", [ | ||
| index("p/tabs/Wiki/WikiFirstPage.tsx"), | ||
| route("/new", "p/tabs/Wiki/WikiNewPage.tsx"), | ||
| route("/:slug", "p/tabs/Wiki/WikiPage.tsx"), | ||
| route("/:slug/edit", "p/tabs/Wiki/WikiPageEdit.tsx"), | ||
| ]), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Drop leading slashes in nested wiki routes.
Within prefix("wiki", ...) and a layout(...), child paths must be relative. Leading / re-anchors them at the app root.
layout("p/tabs/Wiki/Wiki.tsx", [
index("p/tabs/Wiki/WikiFirstPage.tsx"),
- route("/new", "p/tabs/Wiki/WikiNewPage.tsx"),
- route("/:slug", "p/tabs/Wiki/WikiPage.tsx"),
- route("/:slug/edit", "p/tabs/Wiki/WikiPageEdit.tsx"),
+ route("new", "p/tabs/Wiki/WikiNewPage.tsx"),
+ route(":slug", "p/tabs/Wiki/WikiPage.tsx"),
+ route(":slug/edit", "p/tabs/Wiki/WikiPageEdit.tsx"),
]),📝 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.
| layout("p/tabs/Wiki/Wiki.tsx", [ | |
| index("p/tabs/Wiki/WikiFirstPage.tsx"), | |
| route("/new", "p/tabs/Wiki/WikiNewPage.tsx"), | |
| route("/:slug", "p/tabs/Wiki/WikiPage.tsx"), | |
| route("/:slug/edit", "p/tabs/Wiki/WikiPageEdit.tsx"), | |
| ]), | |
| layout("p/tabs/Wiki/Wiki.tsx", [ | |
| index("p/tabs/Wiki/WikiFirstPage.tsx"), | |
| route("new", "p/tabs/Wiki/WikiNewPage.tsx"), | |
| route(":slug", "p/tabs/Wiki/WikiPage.tsx"), | |
| route(":slug/edit", "p/tabs/Wiki/WikiPageEdit.tsx"), | |
| ]), |
🤖 Prompt for AI Agents
apps/cyberstorm-remix/app/routes.ts around lines 27 to 32: nested routes inside
layout currently use leading slashes which re-anchor them to the app root;
remove the leading slashes so child paths are relative (change "/new" → "new",
"/:slug" → ":slug", "/:slug/edit" → ":slug/edit") and keep index(...) as-is so
the wiki routes resolve under the "wiki" prefix.
| --dropdown-item-bg-color--default: hsl(0deg 0 0% / 0); | ||
| --dropdown-item-bg-color--hover: var(--color-nightsky-a7); | ||
| --dropdown-item-icon-color--default: var(--color-text-tertiary); | ||
| --dropdown-item-icon-color--hover: var(--color-text-primary); | ||
| --dropdown-item-text-color--default: var(--color-text-primary); | ||
| --dropdown-item-text-color--hover: var(--color-text-primary); | ||
| --dropdown-item-danger-bg-color--default: #0000; | ||
| --dropdown-item-danger-bg-color--default: hsl(0deg 0 0% / 0); | ||
| --dropdown-item-danger-bg-color--hover: var(--color-accent-red-6); | ||
| --dropdown-item-danger-icon-color--default: var(--color-accent-red-7); | ||
| --dropdown-item-danger-icon-color--hover: #fff; | ||
| --dropdown-item-danger-icon-color--hover: hsl(0deg 0 100%); | ||
| --dropdown-item-danger-text-color--default: var(--color-accent-red-7); | ||
| --dropdown-item-danger-text-color--hover: #fff; | ||
| --dropdown-item-disabled-bg-color--default: #0000; | ||
| --dropdown-item-disabled-bg-color--hover: #0000; | ||
| --dropdown-item-danger-text-color--hover: hsl(0deg 0 100%); | ||
| --dropdown-item-disabled-bg-color--default: hsl(0deg 0 0% / 0); | ||
| --dropdown-item-disabled-bg-color--hover: hsl(0deg 0 0% / 0); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix invalid HSL syntax (missing percentage units for saturation).
Several hsl() values omit the required % for saturation (S), which is invalid in CSS Color 4 space-separated syntax and may be ignored by browsers. Replace hsl(… 0 …) with hsl(… 0% …).
Apply these representative fixes and repeat for similar lines:
- --dropdown-item-bg-color--default: hsl(0deg 0 0% / 0);
+ --dropdown-item-bg-color--default: hsl(0deg 0% 0% / 0);
- --dropdown-item-danger-bg-color--default: hsl(0deg 0 0% / 0);
+ --dropdown-item-danger-bg-color--default: hsl(0deg 0% 0% / 0);
- --dropdown-item-danger-icon-color--hover: hsl(0deg 0 100%);
+ --dropdown-item-danger-icon-color--hover: hsl(0deg 0% 100%);
- --dropdown-item-danger-text-color--hover: hsl(0deg 0 100%);
+ --dropdown-item-danger-text-color--hover: hsl(0deg 0% 100%);
- --dropdown-item-disabled-bg-color--default: hsl(0deg 0 0% / 0);
+ --dropdown-item-disabled-bg-color--default: hsl(0deg 0% 0% / 0);
- --dropdown-item-disabled-bg-color--hover: hsl(0deg 0 0% / 0);
+ --dropdown-item-disabled-bg-color--hover: hsl(0deg 0% 0% / 0);
- --kbd-bg-color--default: hsl(0deg 0 0% / 0);
+ --kbd-bg-color--default: hsl(0deg 0% 0% / 0);
- --tab-nav-item-border-color--default: hsl(0deg 0 0% / 0);
+ --tab-nav-item-border-color--default: hsl(0deg 0% 0% / 0);
- --table-cell-bg-color--default: hsl(0deg 0 100%);
+ --table-cell-bg-color--default: hsl(0deg 0% 100%);
- --table-cell-border-color--default: hsl(0deg 0 100%);
+ --table-cell-border-color--default: hsl(0deg 0% 100%);
- --table-header-bg-color--default: hsl(0deg 0 100%);
+ --table-header-bg-color--default: hsl(0deg 0% 100%);
- --table-header-bg-color--hover: hsl(0deg 0 100%);
+ --table-header-bg-color--hover: hsl(0deg 0% 100%);Also applies to: 184-186, 276-276, 341-342, 356-360
🤖 Prompt for AI Agents
In packages/cyberstorm-theme/src/components/componentsColors.css around lines
175 to 188 (also apply same changes at 184-186, 276, 341-342, and 356-360),
several hsl() color declarations use an invalid syntax by omitting the % unit on
the saturation component (e.g. hsl(0deg 0 0% / 0)); update these to include the
percentage sign for saturation (e.g. hsl(0deg 0% 0% / 0)) so they conform to CSS
Color 4 space-separated hsl() syntax and won’t be ignored by browsers. Ensure
you correct each occurrence of "hsl(... 0 ..." to "hsl(... 0% ..." and similarly
add % to any other saturation values missing the unit.
| {csSize === "textarea" ? ( | ||
| <Input | ||
| {...(fProps as InputTextInputProps)} | ||
| primitiveType={"textInput"} | ||
| {...(fProps as InputTextAreaProps)} | ||
| primitiveType={"textArea"} | ||
| rootClasses={classnames( | ||
| "text-input", | ||
| leftIcon ? "text-input--has-left-icon" : null, | ||
| clearValue ? "text-input--has-clear-value" : null, | ||
| ...componentClasses("text-input", csVariant, csSize, csModifiers) | ||
| )} | ||
| ref={inputRef} | ||
| ref={textareaRef} | ||
| onKeyDown={onEnter} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t overload size to switch element type; fix prop/ref typing.
Using csSize === "textarea" to render a textarea couples “size” with “kind” and breaks typing (props currently extend InputTextInputProps). Prefer a discriminated prop (e.g., as: "input" | "textarea") and update TextInputProps to a union; also change ref to HTMLInputElement | HTMLTextAreaElement.
Minimal code sketch (outside this hunk) to illustrate:
type Common = {
leftIcon?: ReactElement;
rightIcon?: ReactElement;
enterHook?: (value: string | number | readonly string[]) => string | void;
clearValue?: () => void;
csVariant?: TextInputVariants;
csSize?: Exclude<TextInputSizes, "textarea">; // keep size tokens semantic
csModifiers?: TextInputModifiers[];
rootClasses?: string;
ref?: React.Ref<HTMLInputElement | HTMLTextAreaElement>;
};
type TextInputInputProps = Common & Omit<InputTextInputProps, "primitiveType"> & { as?: "input" };
type TextInputTextAreaProps = Common & Omit<InputTextAreaProps, "primitiveType"> & { as: "textarea" };
export type TextInputProps = TextInputInputProps | TextInputTextAreaProps;And in render:
return props.as === "textarea" ? (
<Input {...(fProps as InputTextAreaProps)} primitiveType="textArea" ref={textareaRef} .../>
) : (
<Input {...(fProps as InputTextInputProps)} primitiveType="textInput" ref={inputRef} .../>
);🤖 Prompt for AI Agents
In packages/cyberstorm/src/newComponents/TextInput/TextInput.tsx around lines
80–91, the component incorrectly uses csSize === "textarea" to switch between
input and textarea which couples size with element kind and breaks prop/ref
typing; change the API to a discriminated union (e.g., add an "as" prop with
"input" | "textarea"), update TextInputProps to be a union of Input and TextArea
variants (move csSize to exclude "textarea"), and update forwarded/inner refs to
React.Ref<HTMLInputElement | HTMLTextAreaElement>; then render based on props.as
(casting fProps to the correct InputTextInputProps or InputTextAreaProps) and
pass the corresponding ref (inputRef or textareaRef) and primitiveType.
Summary by CodeRabbit
New Features
Improvements