Skip to content

feat: change navdrop links and sub-links style on hover#463

Closed
gauravtak wants to merge 14 commits intoasyncapi:masterfrom
gauravtak:hover-nav-text-bg-color
Closed

feat: change navdrop links and sub-links style on hover#463
gauravtak wants to merge 14 commits intoasyncapi:masterfrom
gauravtak:hover-nav-text-bg-color

Conversation

@gauravtak
Copy link

@gauravtak gauravtak commented Nov 12, 2024

Description

  • Links in navdrop on small screens change text on hover from (white:cyan-400)
  • Added transition property to sub-links on hover
  • Background of sub-links have rounded borders

Demo
asyncapi-conf-1
asyncapi-conf-2

@netlify
Copy link

netlify bot commented Nov 12, 2024

Deploy Preview for peaceful-ramanujan-288045 ready!

Name Link
🔨 Latest commit 2522eed
🔍 Latest deploy log https://app.netlify.com/sites/peaceful-ramanujan-288045/deploys/67e19b8489dad10008a97d6a
😎 Deploy Preview https://deploy-preview-463--peaceful-ramanujan-288045.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Awesome enhancement contribution @gaurav-2-0-0-2, but i think a nice addition will be to show which link is currently active. wyt?

@gauravtak
Copy link
Author

gauravtak commented Nov 13, 2024

Awesome enhancement contribution @gaurav-2-0-0-2, but i think a nice addition will be to show which link is currently active. wyt?

Yes I was thinking the same thing. will add changes to this PR

@ashmit-coder
Copy link
Contributor

Hey @gaurav-2-0-0-2 The second hover effects for mobile screens would look better as way of indicating where the user is as said by Ace. But I dont see a use of them in mobile view, as after clicking the links the user would simply be redirected.

Copy link
Contributor

@ashmit-coder ashmit-coder left a comment

Choose a reason for hiding this comment

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

Remove the changes in package-lock.json

@gauravtak
Copy link
Author

Remove the changes in package-lock.json

do i need to remove package-lock.json when making a PR

@ashmit-coder
Copy link
Contributor

Remove the changes in package-lock.json

do i need to remove package-lock.json when making a PR

Just dont have any changes in that file

@gauravtak
Copy link
Author

Hey @gaurav-2-0-0-2 The second hover effects for mobile screens would look better as way of indicating where the user is as said by Ace. But I dont see a use of them in mobile view, as after clicking the links the user would simply be redirected.

and the page re renders and the navdrop set to what it was, do we need to highlight the sub menus here or will it be an overkill ?

@ashmit-coder
Copy link
Contributor

I don't think it will be an overkill tbh. Wyt @AceTheCreator

@gauravtak
Copy link
Author

Hey, is this enough ?
asyncapi-conf-4

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

LGTM! cc @ashmit-coder

@gauravtak
Copy link
Author

hey is this going to be merged, it is showing some conflicts ?

@ashmit-coder
Copy link
Contributor

hey is this going to be merged, it is showing some conflicts ?

yes @gaurav-2-0-0-2 you have to resolve those conflicts on your local and then push the new code.

Do try not to hurt the conflicting functionality and maintain your changes as well.

If you need help you can ask in slack. The topic you want to look at is git merge conflict

@gauravtak
Copy link
Author

hey @ashmit-coder i resolved the conflicts, made a small change to make the functionality work, is it good to go now ? do suggest if any changes required.

@ashmit-coder
Copy link
Contributor

@gaurav-2-0-0-2 did you make sure not to remove the functionality that was giving conflict.

tell me the name of the file that was causing the conflict.

@gauravtak
Copy link
Author

@gaurav-2-0-0-2 did you make sure not to remove the functionality that was giving conflict.

tell me the name of the file that was causing the conflict.

Yes I made sure to not remove those changes
The file was components/Navbar/navDrop.js

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

LGTM!

cc @ashmit-coder

@AceTheCreator
Copy link
Member

@gaurav-2-0-0-2 kindly resolve the conflict on this PR 🙏🏽

@gauravtak
Copy link
Author

gauravtak commented Dec 17, 2024

hey @AceTheCreator resolved conflicts I think you can merge now

Copy link
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

LGTM!

@AceTheCreator
Copy link
Member

@ashmit-coder, your approval is needed 👍🏾

@AceTheCreator
Copy link
Member

Pinging you on this too @ashmit-coder

@Recxsmacx
Copy link
Contributor

@AceTheCreator rtm or I can contribute?

@AceTheCreator
Copy link
Member

@gaurav-2-0-0-2 kindly resolve the conflicts 🙏🏽

Copy link
Contributor

@ashmit-coder ashmit-coder left a comment

Choose a reason for hiding this comment

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

Hey @gaurav-2-0-0-2 sorry for the delay, if you could resolve the conflcts I will merge it asap.

@AceTheCreator
Copy link
Member

@gauravtak kindly resolve the conflicts

@thulieblack
Copy link
Member

Conflicts where not resolved and no response from the contributor. Re-assigning the issue

@thulieblack thulieblack closed this Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments