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

Navbar made responsive #74

Closed
wants to merge 14 commits into from
Closed

Conversation

parthratra11
Copy link
Contributor

image

Copy link

vercel bot commented Jan 11, 2025

@parthratra11 is attempting to deploy a commit to the Yash Kumar Saini's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Jan 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
leetcode-journal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 8:22am

Copy link
Owner

@yashksaini-coder yashksaini-coder left a comment

Choose a reason for hiding this comment

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

Complete the following changes. @parthratra11

README.md Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Remove changes to the README entirely. Don't include it

}`}
>
{item.label}
</Link>
))}
</div>
<div className="hidden lg:flex items-center space-x-4">
<Link href="/auth/signin" className="text-sm font-medium text-muted-foreground hover:text-primary">
<Link
Copy link
Owner

Choose a reason for hiding this comment

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

Why shifted to new line, ??

{navItems.map((item) => (
<Link
key={item.href}
href={item.href}
className={`text-sm font-medium transition-colors hover:text-primary ${
pathname === item.href ? "text-primary" : "text-muted-foreground"
pathname === item.href
Copy link
Owner

Choose a reason for hiding this comment

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

Shifting to new lines is unnecessary

Copy link
Owner

Choose a reason for hiding this comment

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

This is a Ui library component. There is no need to make changes to this file. Remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is because of the prettier extension i use in vscode, it turns the code into a more readable format.
if it isn't affecting the functionality of the app and is making code more readable then you may accept the PR, else lmk, i'll change it and raise a new PR

Copy link
Contributor Author

@parthratra11 parthratra11 Jan 12, 2025

Choose a reason for hiding this comment

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

@yashksaini-coder also there is no change in ui/shiny-button.tsx, there is just on empty line after line 73, because i might have pressed 'enter' there. Kindly confirm.

@yashksaini-coder yashksaini-coder added good first issue Good for newcomers SWOC officially part of SWOC season 5 Beginner SWOC Level 1 issue Under Review The Pull Request is under Review Changes requested Changes are requested Frontend Related to the Frontend part labels Jan 12, 2025
@yashksaini-coder
Copy link
Owner

@parthratra11 Kindly complete the requested changes first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Beginner SWOC Level 1 issue Changes requested Changes are requested Frontend Related to the Frontend part good first issue Good for newcomers SWOC officially part of SWOC season 5 Under Review The Pull Request is under Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants