Fix #201 : Active class for sidebar #221
Conversation
- Modal is now big and fully responsive - Simplified the styling - changed the design of the "To" autocomplete to the desired one
|
@madhav-relish is attempting to deploy a commit to the Inbox Zero Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes involve updates to the Changes
Assessment against linked issues
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/web/app/(app)/compose/ComposeEmailForm.tsx (4 hunks)
- apps/web/components/Modal.tsx (1 hunks)
- apps/web/components/SideNav.tsx (3 hunks)
Additional context used
Biome
apps/web/components/SideNav.tsx
[error] 301-301: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
Additional comments not posted (6)
apps/web/components/Modal.tsx (1)
64-79: LGTM!The changes simplify the styling logic by reducing redundancy in class application while maintaining the conditional styling based on the
propsattributes.The code changes are approved.
apps/web/app/(app)/compose/ComposeEmailForm.tsx (4)
3-8: LGTM!The import statements have been modified to include
ComboboxInput, which enhances modularity.The code changes are approved.
174-179: LGTM!The additional classes for styling improve the visual layout and user interaction.
The code changes are approved.
193-195: LGTM!The simplified class for
ComboboxInputcontributes to a cleaner input style without borders.The code changes are approved.
303-303: LGTM!The updated class for the main container adds a more defined structure to the component.
The code changes are approved.
apps/web/components/SideNav.tsx (1)
337-342: LGTM!The changes ensure that the links in the sidebar are now aware of and include the current search parameters, improving navigation experience.
The code changes are approved.
apps/web/components/SideNav.tsx
Outdated
| const searchParams = useSearchParams(); | ||
| const activePath = "?" + searchParams.toString(); // "?" is added to match the href strings of NavItems |
There was a problem hiding this comment.
Use template literals for string concatenation.
Template literals are preferred over string concatenation for better readability and performance.
Apply this diff to use template literals:
-const activePath = "?" + searchParams.toString();
+const activePath = `?${searchParams.toString()}`;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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const searchParams = useSearchParams(); | |
| const activePath = "?" + searchParams.toString(); // "?" is added to match the href strings of NavItems | |
| const searchParams = useSearchParams(); | |
| const activePath = `?${searchParams.toString()}`; // "?" is added to match the href strings of NavItems |
Tools
Biome
[error] 301-301: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/web/components/SideNav.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/web/components/SideNav.tsx
| {selectedEmailAddressses.map((emailAddress) => ( | ||
| <Badge | ||
| key={emailAddress} | ||
| variant="outline" | ||
| className="mr-1.5" | ||
| className="h-8 rounded-md border-primary bg-primary text-white" |
There was a problem hiding this comment.
mostly not using x-primary but the color instead. sometimes primary appears in codebase but it's rare and i'd prefer to stay consistent with colors here
apps/web/components/SideNav.tsx
Outdated
| @@ -298,6 +298,9 @@ export function SideNav(props: { | |||
| function Sidebar(props: { isMobile: boolean }) { | |||
| const path = usePathname(); | |||
| const showMailNav = path === "/mail" || path === "/compose"; | |||
| const searchParams = useSearchParams(); | |||
| // "?" is added to match the href strings of NavItems | |||
| const activePath = `?${showMailNav && searchParams.toString() ? searchParams.toString() : "type=inbox"}`; // if we are at "/mail" only, then the "Inbox" item will be highlighted | |||
There was a problem hiding this comment.
Something like this would be cleaner and cover cases where there are other searchParams:
const params = new URLSearchParams(searchParams);
params.set(searchParam, value);There was a problem hiding this comment.
This has also been changed for better readability and tested as well!
- Color name is now specified rather than using bg-primary etc - Fixed the layout change of demo video modal on the landing page - Made sidebar code more readable for highlighted labels
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/web/app/(app)/compose/ComposeEmailForm.tsx (4 hunks)
- apps/web/app/(landing)/home/VideoDemo.tsx (1 hunks)
- apps/web/components/SideNav.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(app)/compose/ComposeEmailForm.tsx
Additional comments not posted (3)
apps/web/app/(landing)/home/VideoDemo.tsx (1)
31-31: Approve layout changes, but verify responsiveness.The changes to the
iframeand its container improve flexibility and responsiveness. However, ensure to test the layout across different screen sizes and devices to confirm that the video presentation remains consistent and visually appealing.The code changes are approved.
Run the following script to verify the layout across different devices:
apps/web/components/SideNav.tsx (2)
301-310: Approve changes, suggest adding explanatory comments.The use of
useSearchParamsandURLSearchParamsto dynamically set theactivePathenhances the sidebar's responsiveness to URL changes. However, consider adding comments to explain the logic behind settingactivePathand its use in theLinkscomponent, especially for developers unfamiliar with these hooks.The code changes are approved.
345-350: Approve dynamic link handling in the sidebar.The dynamic handling of
activePathin theLinkscomponent ensures that the sidebar navigation is contextually relevant and reflects the current URL state. This is a good practice and enhances the user experience by making the navigation more intuitive.The code changes are approved.
|
Thanks! |
Fixes #201
The active tab is highlighted when that particular page is active
Summary by CodeRabbit
New Features
Bug Fixes
Style