Next Release #286
Next Release #286
Conversation
Redesign of landing page and small theme adjustments
…h-dependencies fix(deps): update minor and patch dependencies
WalkthroughThis PR implements a comprehensive UI redesign and refactoring across the application, introducing new modals and visual styling improvements, restructuring the editor landing page with a simplified layout, refactoring navigation components, updating resizable handle styling, and removing deprecated component properties while maintaining functional stability. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
…h-dependencies chore(deps): update minor and patch dependencies
deps: update dependencies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/landing/crate-entry.tsx (1)
49-49: Minor typo in console warning."Cloud" should be "Could" in the warning message.
Suggested fix
- console.warn("Cloud not read crate details", crateId, e) + console.warn("Could not read crate details", crateId, e)
🤖 Fix all issues with AI agents
In `@app/editor/page.tsx`:
- Around line 312-316: Update the onClick handler that calls window.open in
page.tsx to guard against reverse tabnabbing: pass a third argument with
"noopener,noreferrer" to window.open (i.e., window.open(url, "_blank",
"noopener,noreferrer")) or capture the returned window and set its opener to
null (const w = window.open(...); if (w) w.opener = null); change the inline
onClick using window.open(...) accordingly in the component so external links
open safely.
In `@app/globals.css`:
- Around line 248-250: The ::-webkit-scrollbar-thumb rule in globals.css only
sets border-radius and needs an explicit background color to ensure the thumb is
visible and matches your design; update the ::-webkit-scrollbar-thumb selector
to include a background (and optionally a background-color for hover/active
states via ::-webkit-scrollbar-thumb:hover or ::-webkit-scrollbar-thumb:active)
using the project’s standard scrollbar color variables or a specific hex/RGB
value so webkit browsers render a visible, styled thumb.
In `@components/modals/about-modal.tsx`:
- Around line 38-56: The external Link elements (the Link wrapping "Christopher
Raquet" and the Links using GITHUB_REPO, PRIVACY_POLICY, and LEGALS) currently
use target="_blank" without a rel attribute; update each Link component to
include rel="noopener noreferrer" to prevent reverse-tabnabbing (i.e., add
rel="noopener noreferrer" to the Link for Christopher Raquet and the Link
components referencing GITHUB_REPO, PRIVACY_POLICY, and LEGALS).
🧹 Nitpick comments (9)
components/file-explorer/viewers/iframe.tsx (1)
6-9: Consider revoking the blob URL to prevent memory leaks.
URL.createObjectURLallocates memory that isn't automatically released. Whenprops.datachanges or the component unmounts, the previous blob URL should be revoked. Consider usinguseEffectfor cleanup:♻️ Suggested improvement
- const url = useMemo(() => { - if (!props.data) return "" - return URL.createObjectURL(props.data) - }, [props.data]) + const url = useMemo(() => { + if (!props.data) return "" + return URL.createObjectURL(props.data) + }, [props.data]) + + useEffect(() => { + return () => { + if (url) URL.revokeObjectURL(url) + } + }, [url])components/actions/action-buttons.tsx (2)
76-80: Potential className override when props are spread.The hardcoded
className="gap-2"will be overwritten if a consumer passes their ownclassNameprop, since{...cleanProps(props)}is spread after. Consider merging classNames using a utility likecn().♻️ Suggested fix
+import { cn } from "@/lib/utils" + export const ActionDropdownMenuItem = memo(function ActionDropdownMenuItem( props: ComponentProps<typeof DropdownMenuItem> & GenericActionContentProps ) { const action = useAction(props.actionId) return ( - <DropdownMenuItem className="gap-2" onClick={() => action.execute()} {...cleanProps(props)}> + <DropdownMenuItem className={cn("gap-2", props.className)} onClick={() => action.execute()} {...cleanProps(props)}> <GenericActionContent {...props} /> </DropdownMenuItem> ) })
88-92: Same className override issue applies here.Same concern as
ActionDropdownMenuItemabove. Additionally, note thatActionCommandItem(line 101) doesn't have thegap-2class added—verify if this inconsistency is intentional.components/entity-browser/entity-browser.tsx (1)
35-37: Redundant state selector.
setShowPropertyOverviewis retrieved separately here but is also accessible via thestateobject (line 32) that's already being used on line 101 asstate.setShowPropertyOverview. Consider removing this redundant selector.♻️ Suggested fix
- const setShowPropertyOverview = useEntityBrowserSettings( - (store) => store.setShowPropertyOverview - )Then update line 222 to use
state.setShowPropertyOverview:- onExpand={() => setShowPropertyOverview(true)} - onCollapse={() => setShowPropertyOverview(false)} + onExpand={() => state.setShowPropertyOverview(true)} + onCollapse={() => state.setShowPropertyOverview(false)}components/nav/nav-sidebar.tsx (2)
60-67: Remove redundanttitleattribute.The
title={name}attribute on the Link creates a native browser tooltip that may conflict with or duplicate the custom Tooltip component, causing a poor UX with two overlapping tooltips.♻️ Suggested fix
- <Link href={href} title={name}> + <Link href={href}>
114-123: External link should open in new tab.The feedback link to GitHub issues is an external URL. Consider adding
target="_blank"andrel="noopener noreferrer"for better UX (keeps the editor open) and security.♻️ Suggested fix
- <Link href={"https://github.com/kit-data-manager/NovaCrate/issues"}> + <Link + href={"https://github.com/kit-data-manager/NovaCrate/issues"} + target="_blank" + rel="noopener noreferrer" + >components/modals/about-modal.tsx (2)
3-23: Avoidnext/dist/...private API for basePath handling.
next/dist/client/add-base-pathis an internal API and may break on upgrades. If basePath handling is needed, prefer a public approach (or rely onnext/imagebasePath behavior) and keep it consistent across the app.♻️ Possible simplification (verify basePath behavior first)
-import { addBasePath } from "next/dist/client/add-base-path" @@ - src={addBasePath("/novacrate-nobg.svg")} + src="/novacrate-nobg.svg"
5-31: Consider avoiding fullpackage.jsonin the client bundle.Importing
package.jsonin a client component can pull the entire file into the JS payload. A small build-time constant (e.g., an env var or a dedicatedversionexport) keeps the bundle lean.app/editor/page.tsx (1)
230-235: Align logo asset path handling with the chosen basePath strategy.This uses a raw
/novacrate-nobg.svgpath while the About modal prefixes basePath. If you deploy withbasePath, pick a single, public approach and reuse it to avoid broken assets.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.