Skip to content
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

Web/fix intial render #1120

Merged
merged 5 commits into from
Dec 15, 2024
Merged

Web/fix intial render #1120

merged 5 commits into from
Dec 15, 2024

Conversation

suddenlyGiovanni
Copy link
Owner

This pull request includes several changes across multiple files to improve code consistency and readability. The most important changes involve refactoring components to remove unnecessary hooks and memoization, and updating imports for better type management.

Refactoring components:

  • apps/web/src/shell/header.tsx: Refactored the Header component to remove the memo and useCallback hooks, and directly define the handleMobileNavigationClick function. Simplified the rendering of list items by moving the routes.map logic directly into the JSX. [1] [2] [3]

  • packages/ui/src/components/mode-toggle/mode-toggle.tsx: Refactored the ModeToggle component to remove the memo and useMemo hooks, and replaced them with straightforward logic. Introduced a makeToggleHandler function to handle theme changes more cleanly. [1] [2] [3]

Updating imports:

Refactor to use useRouteLoaderData for more accurate data scoping.
Ensures data retrieval is tied specifically to the 'root' route,
improving clarity and code reliability.
Replaced memoized Header component with a standard function for
clarity and maintainability. Removed unnecessary useCallback and
useMemo hooks, simplifying the component logic. This change improves
code readability without impacting functionality.
Refactor `ModeToggle` component to improve readability and
performance. Replaced `useMemo` usage with direct comparison logic
for theme detection. Introduced a reusable `makeToggleHandler`
function for cleaner event handling.
Copy link

changeset-bot bot commented Dec 15, 2024

⚠️ No Changeset found

Latest commit: 3590132

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Provide clear instructions for scroll management and script tags,
including nonce requirements for content security policies. These
comments improve clarity for future developers and ensure proper
implementation guidelines.
Replace custom document and main components with shared layout
utilities.
This reduces redundancy and ensures consistency across the app by
relying on a centralized layout structure.
Related files `document.tsx` and `main.tsx` were removed as they are
no longer needed.
@suddenlyGiovanni suddenlyGiovanni marked this pull request as ready for review December 15, 2024 01:32
@suddenlyGiovanni suddenlyGiovanni merged commit 0875cee into main Dec 15, 2024
19 checks passed
@suddenlyGiovanni suddenlyGiovanni deleted the web/fix-intial-render branch December 15, 2024 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant