Refactor UI in restaurant detail page #27
Conversation
WalkthroughThe recent updates encompass various functional and stylistic adjustments across multiple components and pages. Key changes include modifying 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (9)
website/pages/category/restaurant.tsx (1)
Line range hint
51-134: Add keyboard event handlers to complement theonClickevents for better accessibility.- <div onClick={() => openBottomSheet(item.name, data.dishes)} className="sm:hidden cursor-pointer"> + <div onClick={() => openBottomSheet(item.name, data.dishes)} onKeyPress={() => openBottomSheet(item.name, data.dishes)} tabIndex="0" className="sm:hidden cursor-pointer">admin/pages/invited-members/add/index.tsx (1)
Line range hint
126-132: Add alternative text to SVG elements to enhance accessibility.- <svg width="13" height="13" viewBox="0 0 13 13" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg width="13" height="13" viewBox="0 0 13 13" fill="none" xmlns="http://www.w3.org/2000/svg" aria-label="Error Icon">Also applies to: 215-221
website/pages/restaurants/[restaurant]/menus/[menu].tsx (4)
Line range hint
37-37: Simplify the conditional expression for clarity.- const [isDishesLoading, setIsDishesLoading] = useState<boolean>(menus ? false : true); + const [isDishesLoading, setIsDishesLoading] = useState<boolean>(!!menus);
Line range hint
103-110: Consider usingfor...offor better performance and readability.- entries.forEach((entry) => { + for (const entry of entries) { if (entry.isIntersecting) { setNumDivsToRender((prevNumDivs) => Math.min(prevNumDivs + 1, menusData?.length) ); } - }); + }
Line range hint
42-42: Remove unnecessary dependencies from useEffect hooks.- }, [suffix, menuSuffix, menusData, menus, isDishesLoading, menuDishesState]); + }, [suffix, menuSuffix, menusData, menus, menuDishesState]); - }, [carouselRef, numDivsToRender, menusData?.length]); + }, [numDivsToRender, menusData?.length]);Also applies to: 95-95
Line range hint
225-229: Specify the button type to avoid unexpected form submissions.- <button onClick={goBack} className="..."> + <button type="button" onClick={goBack} className="...">admin/pages/admins/index.tsx (3)
Line range hint
98-104: Add alternative text to SVG elements for accessibility.- <svg ...> + <svg ... aria-label="Edit"> - <svg ...> + <svg ... aria-label="Delete"> - <svg ...> + <svg ... aria-label="Delete">Also applies to: 167-174, 194-201
Line range hint
62-62: Include all dependencies in useEffect to ensure correct behavior.- }, [currentPage]); + }, [currentPage, fetchAdmins, fetchCountAdmins]);
Line range hint
186-193: Ensure the button type is explicitly defined to prevent form submission.- <button onClick={() => confirm("Are you sure you want to delete this admin?") ? deleteRecord(admin.id) : ""}> + <button type="button" onClick={() => confirm("Are you sure you want to delete this admin?") ? deleteRecord(admin.id) : ""}>Tools
Biome
[error] 186-193: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- admin/pages/admins/index.tsx (3 hunks)
- admin/pages/invited-members/add/index.tsx (1 hunks)
- website/components/BottomSheet/index.tsx (1 hunks)
- website/components/Reel/index.tsx (2 hunks)
- website/components/SkeletonPlaceholders/MenuDish.tsx (1 hunks)
- website/pages/category/restaurant.tsx (5 hunks)
- website/pages/restaurants/[restaurant]/categories/[category].tsx (1 hunks)
- website/pages/restaurants/[restaurant]/menus/[menu].tsx (1 hunks)
Additional context used
Biome
website/components/BottomSheet/index.tsx
[error] 50-53: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or resetwebsite/components/Reel/index.tsx
[error] 50-57: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
[error] 26-26: This hook specifies more dependencies than necessary: dishesData (lint/correctness/useExhaustiveDependencies)
Outer scope values aren't valid dependencies because mutating them doesn't re-render the component.
[error] 38-38: This hook specifies more dependencies than necessary: carouselRef (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
website/pages/category/restaurant.tsx
[error] 118-123: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
admin/pages/invited-members/add/index.tsx
[error] 75-75: This var should be declared at the root of the enclosing function. (lint/correctness/noInnerDeclarations)
The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.
[error] 126-132: Alternative text title element cannot be empty (lint/a11y/noSvgWithoutTitle)
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
[error] 215-221: Alternative text title element cannot be empty (lint/a11y/noSvgWithoutTitle)
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
website/pages/restaurants/[restaurant]/categories/[category].tsx
[error] 48-48: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 53-53: This hook does not specify all of its dependencies: categoriesState.filter (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 53-53: This hook does not specify all of its dependencies: categoriesState.some (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 53-53: This hook does not specify all of its dependencies: dispatch (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
[error] 53-53: This hook does not specify all of its dependencies: dishesData (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 53-53: This hook does not specify all of its dependencies: categoriesState.length (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
This dependency is not specified in the hook dependency list.
[error] 53-53: This hook does not specify all of its dependencies: categoryDishesState.filter (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 207-211: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or resetwebsite/pages/restaurants/[restaurant]/menus/[menu].tsx
[error] 37-37: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
[error] 103-110: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
[error] 42-42: This hook specifies more dependencies than necessary: isDishesLoading (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
[error] 95-95: This hook specifies more dependencies than necessary: carouselRef (lint/correctness/useExhaustiveDependencies)
This dependency can be removed from the list.
[error] 225-229: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or resetadmin/pages/admins/index.tsx
[error] 98-104: Alternative text title element cannot be empty (lint/a11y/noSvgWithoutTitle)
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
[error] 167-174: Alternative text title element cannot be empty (lint/a11y/noSvgWithoutTitle)
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
[error] 194-201: Alternative text title element cannot be empty (lint/a11y/noSvgWithoutTitle)
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
[error] 62-62: This hook does not specify all of its dependencies: fetchAdmins (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 62-62: This hook does not specify all of its dependencies: fetchCountAdmins (lint/correctness/useExhaustiveDependencies)
This dependency is not specified in the hook dependency list.
[error] 186-193: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
Additional comments not posted (6)
website/components/SkeletonPlaceholders/MenuDish.tsx (1)
1-1: LGTM! The addition of thestyleprop enhances the flexibility of theMenuDishSkeletoncomponent.Also applies to: 5-5
website/components/BottomSheet/index.tsx (1)
34-40: ThehandleClosefunction and its integration in theonClickhandler are well-implemented, enhancing the component's functionality.Also applies to: 51-51
website/components/Reel/index.tsx (1)
86-95: The conditional rendering and style adjustments based on the screen height are well-implemented, enhancing the responsiveness of theReelscomponent.Also applies to: 118-129
website/pages/category/restaurant.tsx (1)
Line range hint
51-134: The changes in the rendering logic and the removal of unused imports are well-handled, improving the code quality and performance.admin/pages/invited-members/add/index.tsx (1)
99-99: The simplification of the email subject is a good improvement for clarity.website/pages/restaurants/[restaurant]/categories/[category].tsx (1)
215-215: The removal of the word "dishes" simplifies the display text, aligning it with the intended changes.
| const handleClose = () => { | ||
| if (window.history.state?.bottomSheetOpen) { | ||
| window.history.back(); | ||
| } else { | ||
| onClose(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Ensure to specify the button type to prevent unintended form submissions.
- <button onClick={handleClose} className="...">
+ <button type="button" onClick={handleClose} className="...">Also applies to: 51-51
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.
| const handleClose = () => { | |
| if (window.history.state?.bottomSheetOpen) { | |
| window.history.back(); | |
| } else { | |
| onClose(); | |
| } | |
| }; | |
| const handleClose = () => { | |
| if (window.history.state?.bottomSheetOpen) { | |
| window.history.back(); | |
| } else { | |
| onClose(); | |
| } | |
| }; | |
| // Assuming this is the button element in the component | |
| <button type="button" onClick={handleClose} className="..."> |
| {isLoading ? ( | ||
| <MenuDishSkeleton | ||
| classes="reel" | ||
| style={{ | ||
| height: screenHeight != 0 ? screenHeight + "px" : "100vh", | ||
| }} | ||
| /> | ||
| ) : ( | ||
| "" | ||
| )} |
There was a problem hiding this comment.
Optimize the iteration by replacing forEach with for...of for better performance.
- entries.forEach((entry) => {
+ for (const entry of entries) {Also applies to: 118-129
Committable suggestion was skipped due to low confidence.
Summary by CodeRabbit
Bug Fixes
idparameter in the AdminsPage.Improvements
UI Updates