-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: updated mobile header #1004
Conversation
Signed-off-by: Adithya Krishna <adithya@documenso.com>
WalkthroughThe recent updates focus on refining type imports and enhancing UI components across marketing and dashboard applications. Changes include using Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Files skipped from review as they are similar to previous changes (2)
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 (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
Thank you for following the naming conventions for pull request titles! 💚🚀 |
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
…at/mobile-header
Please make sure build and CI passes before requesting reviews. |
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
packages/ui/primitives/avatar.tsx
Outdated
@@ -65,7 +65,8 @@ const AvatarWithText = ({ | |||
primaryText, | |||
secondaryText, | |||
rightSideComponent, | |||
showText = true, | |||
// Optional class to hide/show the text beside avatar |
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.
Put this in the type, not here
packages/ui/primitives/avatar.tsx
Outdated
@@ -75,7 +76,7 @@ const AvatarWithText = ({ | |||
</Avatar> | |||
|
|||
<div | |||
className={cn('hidden flex-col text-left text-sm font-normal lg:flex', showText && 'flex')} | |||
className={cn('hidden flex-col text-left text-sm font-normal lg:flex', textSectionClassName)} |
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.
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.
@dguyen Actually for mobile, the text is hidden and for desktop screens it is shown according to the designs
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.
@adithyaakrishna can you please link me the designs
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.
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.
@adithyaakrishna could you please compare the image I posted and the design you linked?
The text next to the avatars within the dialog is not visible. You have inadvertently broken all usage of this component on mobile due to hardcoding this hidden class onto the avatar.
I asked you to add a class prop for the avatar text section so you can hide it only for the header.
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.
@dguyen My bad, I missed the text next to the Avatars on the dropdown. I've added the classProp and also removed the hardcoded className
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
…umenso into feat/mobile-header
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
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 Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- apps/web/src/components/(dashboard)/layout/menu-switcher.tsx (1 hunks)
- packages/ui/primitives/avatar.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/(dashboard)/layout/menu-switcher.tsx
- packages/ui/primitives/avatar.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.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- apps/web/src/components/(dashboard)/layout/menu-switcher.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/web/src/components/(dashboard)/layout/menu-switcher.tsx
…at/mobile-header
…umenso into feat/mobile-header
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
Signed-off-by: Adithya Krishna <aadithya794@gmail.com>
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
Actionable comments outside the diff hunks (7)
SIGNING.md (7)
Line range hint
34-34
: Specify the language for fenced code blocks.- ``` + ```bashAdding a language specification to the fenced code blocks improves readability and syntax highlighting in markdown renderers.
Line range hint
25-27
: Use hyphens in compound adjectives.- production ready docker image + production-ready docker imageWhen compound adjectives precede a noun, they should be hyphenated to improve readability and prevent misinterpretation.
Line range hint
29-29
: Start sentences with an uppercase letter.- cd into `docker` directory + Cd into the `docker` directoryEnsuring that sentences start with an uppercase letter follows standard writing conventions and improves the professionalism of the documentation.
Line range hint
40-40
: Correct the contraction to enhance clarity.- Let's you run the container in background + Lets you run the container in the background"Let's" is a contraction for "let us," which is inappropriate in this context. "Lets" is the correct form for allowing or permitting something.
Line range hint
41-41
: Capitalize only proper nouns.- Second half is the app port + second half is the app portIn lists or mid-sentence, avoid capitalizing words unless they are proper nouns to maintain consistency and readability.
Line range hint
41-41
: Add a missing preposition for clarity.- you can change the first half anything you want + you can change the first half to anything you wantAdding the preposition "to" clarifies the intended meaning and ensures grammatical correctness.
Line range hint
42-42
: Correct the contraction to enhance clarity.- Volume let's you persist the data + Volume lets you persist the dataSimilar to a previous correction, "let's" should be replaced with "lets" for grammatical accuracy.
Description:
Summary by CodeRabbit
New Features
showText
property to theMenuSwitcher
component to control text visibility.textSectionClassName
property to theAvatarWithText
component for conditional text section styling.CommandDialog
andDialogContent
components with new positioning and styling properties.Style Updates
Hero
component for various screen sizes.Widget
component.SheetContent
element inMobileNavigation
and adjusted footer layout.Documentation
SIGNING.md
.Refactor