landing redesign - #1481
Conversation
Test this pull request on windows-latestDownload the correct version for your architecture: |
Test this pull request on macos-latestDownload the correct version for your architecture:Click here if you don't know which version to downloadFor running this unsigned version of the app, you will need to run the xattr command on it:
|
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review: #1481 — landing redesign
Branch: landing-redesign → main
Files changed: 30 (+1075 −451)
CI: ✅ All checks passing (lint, typecheck, unit tests, E2E mac, builds Windows/macOS)
Summary
Comprehensive redesign of the Creator Hub landing page. Replaces the old card-based layout (SignInCard, ScenesCard, LearnCard, FeedbackCard) with a modern row-based layout (MyScenesRow, TemplatesRow, LearnRow). Adds a LogoMenu dropdown in the navbar (About, Check for Updates, Settings, Report Bug, Help submenu), a CreateButton split-button, and a standalone About modal. The old navbar action buttons (Report Issue, Help icon, Settings icon) and ConnectionStatusIndicator are removed.
Verdict: ✅ APPROVE
No P0 or P1 issues found. The code is well-structured, correctly typed, and the Electron security model is respected. Several P2 improvement suggestions below.
Specific Review Areas (per requester)
1. Auto-update flow (Electron) ✅
The chain Navbar.handleCheckForUpdates → AppSettings(initialTab=ABOUT, autoCheckForUpdates=true) → AboutTab → UpdateSettings(autoCheck=true) → useEffect fires handleCheckForUpdates is correct. handleCheckForUpdates is stable (deps: [dispatch], dispatch is stable). The useEffect fires once when the modal opens with autoCheck=true. handleCloseSettings properly resets autoCheckForUpdates to false, preventing re-triggers on subsequent opens.
2. useEffect loops ✅
UpdateSettings:useEffect(() => { if (autoCheck) handleCheckForUpdates(); }, [autoCheck, handleCheckForUpdates])— safe, both deps are stable.AppSettings:useEffect(() => { if (open) setActiveTab(initialTab); }, [open, initialTab])— safe,initialTabonly changes before the modal opens.- No infinite loop risk in any effect.
3. Link injection ✅
- All URLs passed to
misc.openExternalcome from trusted sources: config files (HELP_FAQ_URL,CONTACT_SUPPORT_URL,DISCORD_URL,REPORT_ISSUES_URL) or hardcoded arrays (VIDEOS,DOCS). misc.openExternalin preload validates withisUrl()before IPC, rejecting non-URL inputs.- Template thumbnails use
STUDIOS_ADMIN_URL(first-party, in the security restrictions allowlist). - YouTube thumbnail URLs are built from hardcoded video IDs in
resources.ts. backgroundImage: url(${imageUrl})— set via React'sstyleprop (DOM property assignment), not raw CSS. Sources are all trusted (base64 project thumbnails, static imports, first-party CDN URLs).- No user-controllable data reaches
shell.openExternalor CSS injection sinks.
4. Button clickability ✅
CreateButton: MUIButtonGroup+Menu+MenuItem— fully interactive, properdata-testid,aria-haspopup,aria-expanded.LogoMenu: MUIMenu+MenuItemwith proper refs, submenu pattern with hover/click.LearnTabbuttons: native<button>elements.LogoButton: native<button>with:focus-visiblestyles.
5. Security ✅
- No hardcoded secrets or credentials.
- No new dependencies.
- All external URLs from trusted config or hardcoded data.
STUDIOS_ADMIN_URLis in the Electron security restrictions allowlist.- No
dangerouslySetInnerHTML, no unsanitized user input. - Config URLs in
dev.json/prd.jsonare all first-party Decentraland domains.
6. useCallback / component logic ✅
- All
useCallbackdependency arrays are correct. - Closure factories (
handleClickProject,handleClickTemplate,handleClickResource) are a standard React pattern — they create new closures per.map()call but this is expected and acceptable withROW_SIZE = 4. handleCreateProjectcorrectly capturesnewProjectin its deps.
P2 Findings (non-blocking suggestions)
[P2] Duplicated NewProjectPayload type
packages/creator-hub/renderer/src/components/CreateButton/types.ts and packages/creator-hub/renderer/src/components/HomePage/types.ts define the exact same NewProjectPayload type. Consider extracting to a shared location (e.g. alongside the CreateProject modal types) to avoid drift.
[P2] Duplicated project creation flow
Both CreateButton and TemplatesRow implement identical logic: getAvailableProject() → trim path → setNewProject() → render CreateProject modal → createProject(). This pattern also exists in TemplatesPage. Consider extracting into a useCreateProject hook that encapsulates the state + callbacks + modal rendering.
[P2] docIcons array recreated every render
In LearnRow, the docIcons array containing JSX elements is recreated on every render. Move it outside the component as a module constant (React elements are immutable and safe to reuse).
[P2] React.memo partially ineffective
HomeCard and Row are wrapped in React.memo, but they receive new function references as onClick props on each render (from closure factories in .map()). With only 4 items per row this is negligible, but worth noting — the memoization doesn't prevent re-renders here.
[P2] <video> elements won't play
HomeCard renders <video src={videoUrl} muted loop /> without autoPlay or controls. The video will load but display only the first frame (or nothing). If hover-to-play is intended, onMouseEnter/onMouseLeave handlers to call .play()/.pause() are missing.
[P2] Accessibility: <div> used as interactive elements
HomeCard and HomeRowHeader.Clickable use <div onClick> without role="button", tabIndex={0}, or keyboard event handlers. Consider using <button> (with reset styles) or adding the appropriate ARIA attributes for keyboard/screen-reader users.
[P2] PR title convention
The PR title "landing redesign" doesn't follow the repo's semantic commit format (e.g. feat: redesign landing page). Most recent merged PRs use the type: summary pattern.
Consumer Impact
This is a renderer-only UI change — no public API surface, exported packages, or shared types are affected. No downstream consumer impact.
Reviewed by Jarvis 🤖 · Requested by Gabriel Díaz (<@U03MGHMAJL8>) via Slack
| onClickSignIn: () => void; | ||
| export type NewProjectPayload = { | ||
| name: string; | ||
| path: string; |
There was a problem hiding this comment.
[P2] NewProjectPayload is defined identically in CreateButton/types.ts (lines 1–4) and here. Consider sharing from a single location to prevent drift.
| }, | ||
| [createProject, newProject], | ||
| ); | ||
|
|
There was a problem hiding this comment.
[P2] docIcons is a static array of React elements but is recreated on every render of LearnRow. Since these are just icon components with no dynamic props, move them outside the component as a module-level constant:
| const DOC_ICONS = [ | |
| <RocketLaunchOutlinedIcon />, | |
| <CodeIcon />, | |
| <AccountTreeOutlinedIcon />, | |
| <WidgetsOutlinedIcon />, | |
| ]; |
| </CardActions> | ||
| ) : null} | ||
| </Card> | ||
| <Row |
There was a problem hiding this comment.
[P2] <video src={videoUrl} muted loop /> — without autoPlay or controls, videos will load but not play. If the intent is hover-to-play, consider adding onMouseEnter={(e) => e.currentTarget.play()} and onMouseLeave={(e) => { e.currentTarget.pause(); e.currentTarget.currentTime = 0; }}.
| const handleSeeAll = useCallback(() => navigate('/scenes'), [navigate]); | ||
| const handleClickProject = useCallback( | ||
| (project: Project) => () => runProject(project), | ||
| [runProject], |
There was a problem hiding this comment.
[P2] Accessibility: <div className="HomeCard" onClick={onClick}> is not keyboard-accessible. Consider using <button> with reset styles, or add role="button", tabIndex={0}, and an onKeyDown handler for Enter/Space.
# Conflicts: # packages/creator-hub/renderer/src/config/env/dev.json # packages/creator-hub/renderer/src/modules/utils.ts
|
QA update ✅ Passed: LogoMenu
Top nav
Create flow
Auth
My Scenes
Templates
Learn
Build
🐛 Bug found Scroll position carries over when navigating away from the homepage. ❓ To confirm:
None of the issues are blockers; this is good to merge. |
4 participants
CH Landind Redesign
Redid the landing page based on this Figma
https://www.figma.com/design/SDSx2kigI76sGeCkSIHFx0/%F0%9F%A7%B0-UX-Enhancement-%7C-Creators-Hub-%7C-07-26?node-id=1401-1298&p=f&t=DJUr6FJTbyb6iXLL-0
Things I intentionally didn't include from the design, to keep the changes light:
--
--