-
Notifications
You must be signed in to change notification settings - Fork 3
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
refactor: make drawer component #189
Conversation
WalkthroughThe recent changes involve a significant refactor of the user interface components within the application, primarily focusing on the introduction of a new The Overall, the changes reflect a shift towards a more cohesive and organized component architecture, with a focus on utilizing the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for profound-kringle-30b3e4 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 2
Outside diff range, codebase verification and nitpick comments (1)
src/pages/grade.tsx (1)
22-48
: Empty "Review" link in Drawer component.The "Review" link in the
Drawer
component'spageTabs
prop is currently empty in thesrc/pages/grade.tsx
file. This could lead to navigation issues. Please ensure that this link is correctly set to provide a seamless user experience.
- Location:
src/pages/grade.tsx
, line 23Analysis chain
Proper integration of the Drawer component.
The
Drawer
component is correctly used to manage the layout and navigation of the page. However, ensure that the link for the "Review" page is correctly set as it currently appears empty.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the link for the "Review" page. # Test: Search for the usage of the "Review" link. Expect: A valid URL. rg --type typescript -A 5 $'pageName: "Review", link: ""'Length of output: 93
Script:
#!/bin/bash # Description: Verify the link for the "Review" page with the correct file type. # Test: Search for the usage of the "Review" link. Expect: A valid URL. rg --type tsx -A 5 $'pageName: "Review", link: ""'Length of output: 79
Script:
#!/bin/bash # Description: Verify the link for the "Review" page without specifying the file type. # Test: Search for the usage of the "Review" link. Expect: A valid URL. rg -A 5 $'pageName: "Review", link: ""'Length of output: 415
Tools
Biome
[error] 41-41: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- src/components/Applicant.tsx (1 hunks)
- src/components/Drawer.tsx (1 hunks)
- src/components/NavBar.tsx (2 hunks)
- src/pages/apply.tsx (1 hunks)
- src/pages/checkin.tsx (2 hunks)
- src/pages/dashboard.tsx (2 hunks)
- src/pages/grade.tsx (2 hunks)
- src/pages/killswitch.tsx (1 hunks)
- src/pages/me.tsx (3 hunks)
- src/pages/scanner.tsx (2 hunks)
- src/pages/schedule.tsx (2 hunks)
- src/pages/welcome.tsx (1 hunks)
Files skipped from review due to trivial changes (4)
- src/components/Applicant.tsx
- src/pages/apply.tsx
- src/pages/killswitch.tsx
- src/pages/schedule.tsx
Additional context used
Biome
src/pages/grade.tsx
[error] 41-41: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
Additional comments not posted (10)
src/pages/grade.tsx (1)
7-7
: Correct import statement for the Drawer component.The import path and usage are correctly updated to reflect the new location of the
Drawer
component.src/components/NavBar.tsx (1)
1-1
: Removal of the Drawer component is appropriate.The removal aligns with the PR objectives to simplify the
NavBar
and separate concerns. Ensure that all functionalities previously handled by theDrawer
are now appropriately managed elsewhere in the application.Verification successful
Functionalities previously handled by the Drawer component are appropriately integrated elsewhere.
The use of
useSession
anduseRouter
across multiple files confirms that session management and routing functionalities are managed throughout the application. This supports the decision to remove theDrawer
component from theNavBar
, aligning with the PR's objectives to simplify and separate concerns.
src/components/NavBar.tsx
src/pages/scanner.tsx
src/pages/dashboard.tsx
src/pages/me.tsx
src/pages/apply.tsx
src/pages/roles.tsx
src/pages/checkin.tsx
src/pages/login.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of functionalities previously handled by the Drawer. # Test: Search for the functionalities like theme toggle and session management. Expect: These functionalities are managed elsewhere. rg --type typescript -A 5 $'useSession|useRouter'Length of output: 85
Script:
#!/bin/bash # Description: Verify the integration of functionalities previously handled by the Drawer. # Test: Search for the functionalities like theme toggle and session management. Expect: These functionalities are managed elsewhere. rg --type ts -A 5 $'useSession|useRouter'Length of output: 11805
src/pages/welcome.tsx (1)
4-4
: Correct update to the import statement for the Drawer component.The change from a named import to a default import reflects the new structure of the
Drawer
component being in its own module. The import path is correctly updated.src/components/Drawer.tsx (1)
1-12
: Well-organized imports and interface definition.The import statements are appropriate for the component's functionality, and the
PageTab
interface is correctly defined.src/pages/me.tsx (1)
Line range hint
1-13
: Appropriate use of imports and the Drawer component.The import statements are relevant and well-organized. The usage of the
Drawer
component to centralize navigation and layout is correctly implemented, enhancing the maintainability and modularity of the component.Also applies to: 50-111
src/pages/checkin.tsx (1)
1-12
: Appropriate use of imports and the Drawer component.The import statements are relevant and well-organized. The usage of the
Drawer
component to centralize navigation and layout is correctly implemented, enhancing the maintainability and modularity of the component.Also applies to: 201-218
src/pages/scanner.tsx (2)
510-547
: Approve the usage of Drawer and suggest verification of role-based logic.The
Drawer
component is used effectively to encapsulate the main content and navigation. Verify that all role-based views are correctly rendered and accessible as intended.
13-13
: Verify the integration of the Drawer component.The import of
Drawer
from../components/Drawer
replaces the previously usedBackground
andNavBar
. Ensure thatDrawer
is correctly integrated and all functionalities previously handled byBackground
andNavBar
are now adequately covered byDrawer
.Verification successful
Drawer component is correctly integrated in
scanner.tsx
.The
Drawer
component is imported and used with thepageTabs
prop, effectively replacing the previous layout components. This confirms the intended centralization of drawer-related code.
src/pages/scanner.tsx
:Drawer
is used withpageTabs
for navigation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the Drawer component in the scanner.tsx file. # Test: Search for the usage of `Drawer` in the scanner.tsx file. Expect: Proper integration and replacement of old components. rg --type tsx -A 5 $'Drawer' src/pages/scanner.tsxLength of output: 77
Script:
#!/bin/bash # Description: Verify the integration of the Drawer component in the scanner.tsx file. # Locate the scanner.tsx file and search for the usage of `Drawer`. fd -e tsx -x rg 'Drawer' {}Length of output: 978
Script:
#!/bin/bash # Description: Verify the integration of the Drawer component specifically in the scanner.tsx file. # Search for the usage of `Drawer` in the scanner.tsx file and include file paths in the output. rg 'Drawer' src/pages/scanner.tsx -A 5 --with-filenameLength of output: 1001
src/pages/dashboard.tsx (2)
17-17
: Verify the integration of the Drawer component.The import of
Drawer
from../components/Drawer
replaces the previously usedBackground
,NavBar
, andThemeToggle
. Ensure thatDrawer
is correctly integrated and all functionalities previously handled by these components are now adequately covered byDrawer
.Verification successful
Verified: The Drawer component is correctly integrated.
The
Drawer
component is used to wrap the main content and footer insrc/pages/dashboard.tsx
, indicating it is effectively replacing the functionalities of the removed components (Background
,NavBar
, andThemeToggle
). The integration appears consistent with the intended layout changes.
- The
Drawer
component is used withpageTabs
for navigation, likely replacingNavBar
.- The layout management seems to be handled by
Drawer
, replacingBackground
andThemeToggle
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the Drawer component in the dashboard.tsx file. # Test: Search for the usage of `Drawer` in the dashboard.tsx file. Expect: Proper integration and replacement of old components. rg --type tsx -A 5 $'Drawer' src/pages/dashboard.tsxLength of output: 79
Script:
#!/bin/bash # Description: Verify the integration of the Drawer component in the dashboard.tsx file. # Test: Search for the usage of `Drawer` in the dashboard.tsx file. Expect: Proper integration and replacement of old components. rg 'Drawer' src/pages/dashboard.tsx -A 5Length of output: 561
466-473
: Approve the usage of Drawer and suggest verification of status-based logic.The
Drawer
component is used effectively to encapsulate the main content and navigation. Verify that all status-based views are correctly rendered and accessible as intended.Verification successful
Status-based logic in the Dashboard component is correctly implemented.
The
Dashboard
component effectively queries and uses the application status to render the appropriate content. The logic for handling different statuses appears to be functioning as intended. No issues were found with the status-based logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify status-based logic in the Dashboard component. # Test: Search for status-based logic handling in the Dashboard component. Expect: Correct rendering and accessibility for different statuses. rg --type tsx -A 5 $'status' src/pages/dashboard.tsxLength of output: 79
Script:
#!/bin/bash # Locate the dashboard.tsx file and search for status-based logic within it. # Find the dashboard.tsx file fd --type f --name 'dashboard.tsx' --exec rg 'status' {}Length of output: 307
Script:
#!/bin/bash # Locate the dashboard.tsx file and search for status-based logic within it. # Find the dashboard.tsx file and search for status-based logic fd 'dashboard.tsx' -x rg 'status' {}Length of output: 356
const { status, count } = value; | ||
return ( | ||
<> | ||
{status}: {count} <br /> |
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.
Add a key property to iterable elements.
The React fragment inside the map function at line 41 should have a unique key property to optimize rendering and update processes.
Suggest adding a key as follows:
- <>
+ <React.Fragment key={i}>
Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 41-41: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
export const Drawer = ({ | ||
children, | ||
pageTabs, | ||
}: { | ||
children: JSX.Element[] | JSX.Element; | ||
pageTabs?: PageTab[]; | ||
}) => { | ||
const { data: session } = useSession(); | ||
const router = useRouter(); | ||
|
||
const drawer = useRef<HTMLInputElement>(null); | ||
|
||
return ( | ||
<> | ||
<Background /> | ||
<div className="drawer drawer-end relative h-full min-h-screen w-full overflow-x-hidden font-montserrat"> | ||
<input | ||
id="my-drawer-3" | ||
type="checkbox" | ||
className="drawer-toggle" | ||
ref={drawer} | ||
/> | ||
|
||
<div className="drawer-content flex flex-col"> | ||
<NavBar /> | ||
{children} | ||
</div> | ||
|
||
<div className="drawer-side md:hidden z-50 "> | ||
<label | ||
htmlFor="my-drawer-3" | ||
className="drawer-overlay md:hidden" | ||
></label> | ||
|
||
<div className="menu h-full w-80 flex-row content-between overflow-y-auto bg-white p-4 dark:bg-[#1F1F1F] md:hidden"> | ||
<ul className="w-full"> | ||
<button | ||
className="btn btn-square btn-ghost drawer-button" | ||
onClick={() => { | ||
if (drawer.current) { | ||
drawer.current.checked = false; | ||
} | ||
}} | ||
> | ||
<svg | ||
xmlns="http://www.w3.org/2000/svg" | ||
width="24" | ||
height="24" | ||
viewBox="0 0 24 24" | ||
> | ||
<path | ||
fill="currentColor" | ||
fillRule="evenodd" | ||
d="m6.5 17.5l8.25-5.5L6.5 6.5l1-1.5L18 12L7.5 19z" | ||
/> | ||
</svg> | ||
</button> | ||
{pageTabs?.map(({ pageName, link }, i) => ( | ||
<li key={i}> | ||
<Link className="mx-2 my-2 text-base font-bold" href={link}> | ||
{pageName} | ||
</Link> | ||
</li> | ||
))} | ||
</ul> | ||
|
||
<div className="mx-1 mb-2 flex w-full items-center justify-between "> | ||
<ThemeToggle /> | ||
{session ? ( | ||
<div> | ||
<a className="font-sub mx-2.5 text-sm"> | ||
Hi,{" "} | ||
<strong className="font-bold">{session?.user?.name}</strong> | ||
</a> | ||
<button | ||
onClick={() => signOut()} | ||
className="font-sub rounded bg-primary px-2.5 py-2.5 text-sm font-bold text-white dark:text-gray-300" | ||
> | ||
Sign Out | ||
</button> | ||
</div> | ||
) : ( | ||
<button | ||
className="btn btn-primary text-white" | ||
onClick={() => router.push("/login")} | ||
> | ||
Sign In | ||
</button> | ||
)} | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
</> | ||
); | ||
}; | ||
|
||
export default Drawer; |
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.
Well-structured Drawer component with effective use of React features.
The component is well-structured, making good use of React hooks, refs, and conditional rendering. The use of useSession
for session management and useRouter
for navigation is appropriate. The ref usage for controlling the drawer's state is a good practice.
Suggestion for Improvement:
Consider adding aria-labels
to interactive elements like buttons for better accessibility.
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/pages/grade.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/pages/grade.tsx
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.
Looks great! Some of this work might need more refactor in the future, such as maybe including defaults for pageTabs in the drawer component.
This refactor introduced some bugs with mobile content sizing, it may need to be revisited in the future. |
* feat: updated schedule for DH10 (#179) * chore: setup using google api * chore: use google api * feat: make calendar work * feat: add schedule button --------- Co-authored-by: Arian Ahmadinejad <35879206+arian81@users.noreply.github.com> Co-authored-by: Arian Ahmadinejad <ahmadinejadarian@gmail.com> * chore: hide logout on schedule * chore: add caching * chore: add attendee package button * feat: add event locations to schedule (#182) * decoupled a lot of functionality in schedule.tsx * fixed getDefaultCurrentDate refactor * cleaned up schedule.tsx from merge * added location to the description * removed console.logs * added location to subheading of events * doing some cleanup * removed removeResourceLabel since description is now unviewable * removed removeResourceLabel since description is now unviewable * making pop viewable again because it needs to be * support 5 colour option * fixing tooltip * fix: disable pop up * added circles in agenda view --------- Co-authored-by: Rachelle DeMan <demanr@mcmaster.ca> Co-authored-by: Krish <krish120003@gmail.com> Co-authored-by: Arian Ahmadinejad <ahmadinejadarian@gmail.com> * fix: schedule login hot fix * fix: hotfix update z indexing on mobile, add sign in button * fix: uncomment out appointmentTooltipRender (#184) * decoupled a lot of functionality in schedule.tsx * fixed getDefaultCurrentDate refactor * cleaned up schedule.tsx from merge * added location to the description * removed console.logs * added location to subheading of events * doing some cleanup * removed removeResourceLabel since description is now unviewable * removed removeResourceLabel since description is now unviewable * making pop viewable again because it needs to be * support 5 colour option * fixing tooltip * fix: disable pop up * added circles in agenda view * uncommented out tooltip render --------- Co-authored-by: Rachelle DeMan <demanr@mcmaster.ca> Co-authored-by: Krish <krish120003@gmail.com> Co-authored-by: Arian Ahmadinejad <ahmadinejadarian@gmail.com> * feat: Create LICENSE * chore: cleanup old components (#187) Remove Google Analytics, Logrocket and unused files. Also mark routers for deprecation. * feat: add posthog * refactor: make drawer component (#189) * created Drawer component * decoupled Drawer out of welcome page * decoupled Drawer out of dashboard, grade, me, and scanner pages * change ApplicationTable button text from View Application to View * remove unused files * cleaning up file dependencies * removed navigation file * feat: DH11 Applications (#196) * feat: DH11 application and review tables * fix: update logsnag project * feat: add user to dh11 applications * feat: use DH11 Applications * fix: update routes to refer to dh11 * feat: prisma db migration * fix: remove broken migrationi * feat: change from DH10 to DH11 everywhere * fix: form is submittable * fix: improve errors * fix: don't refetch autofill * fix: more better errors for form * fix: typos * fix: add better errors * fix: more typos * fix: custom socials form handling (#198) * refactor: rearrage files * feat: add react icons * fix: imports * feat: fixes to form, review endpoint and ui update * fix: imports * fix: update routing and style consistency for dh11 (#199) * fix: disable outdated pages * fix: update routing to use DH11 status, capitalize name * fix: update modal styling * fix: readable dark button text color * feat: use same button as dashboard * feat: match dark and light primary colors * fix: restore original background * fix: match text styling with dashboard * fix: routing TS errors * feat: create prisma migration (#200) * fix: MLH form fields and resume are optional, change form persist key for DH11 (#201) * feat: update socials and info about DH11 (#202) * fix: update DH11 socials * fix: update hackathon length * fix: improve UX with link behavior, spelling, education labels and word count (#203) * fix: open links in new tab * fix: spelling of lookout * fix: add optional to education labels * fix: make word count ux better * fix: show word count on grading page instead of words left --------- Co-authored-by: Krish <krish120003@gmail.com> * feat: add resume upload (#204) * feat: DH11 application and review tables * fix: update logsnag project * feat: add user to dh11 applications * feat: use DH11 Applications * fix: update routes to refer to dh11 * feat: prisma db migration * feat: backend for uppy signed url upload * fix: remove unnecessary validation, store at root * fix: cleanup packages * feat: basic upload component * feat: add endpoint for getting resume files * feat: handle uppy upload responses * fix: handle empty string dates * feat: connect uppy to react form * fix: prettier formatting * fix: add missing types * fix: make form mobile friendly again * fix: remove migration * fix: add missing libraries --------- Co-authored-by: Krish120003 <krish120003@gmail.com> * fix: posthog rewrite using netlify * feat: add posthog submission tracking * feat: use netlify redirects for posthog, add identification and apply event capture (#205) * feat: add posthog identify * fix: upgrade posthog * fix: pnpm lock * fix: use posthog suggested event naming * fix: add missing space * feat: add logsnag track for dh11 application * fix: correct host * fix: add posthog on server * fix: use component to identify in posthog * fix: use consistent id --------- Co-authored-by: Arian Ahmadinejad <ahmadinejadarian@gmail.com> --------- Co-authored-by: Krish <krish120003@gmail.com> Co-authored-by: Felix Fong <fongf2@mcmaster.ca> Co-authored-by: Rachelle DeMan <demanr@mcmaster.ca>
Lots of duplicate code for implementing drawers on different pages. This change centralizes all Drawer related code to the Drawer component. Also some minor code cleanup.
Changes
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Drawer
component for improved user navigation and session management.Bug Fixes
Refactor
NavBar
component by removing the drawer functionality.Drawer
component for consistent navigation and improved layout organization.Chores