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

fix: Mobile Nav improvement #5781

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

upsaurav12
Copy link

Description

This PR fixes #5713

Notes for Reviewers
I have added + sign for toggle to show and hide sub-items. I feel this navbar need more improvement in styling also
And it is looking like this

cinnamon-2024-08-04T1256150530.mp4

Signed commits

  • Yes, I signed my commits.

@l5io
Copy link
Contributor

l5io commented Aug 4, 2024

🚀 Preview for commit 118c890 at: https://66af314b7275d763e197843b--layer5.netlify.app

@Ashparshp
Copy link
Contributor

@upsaurav12, can the subitem be enhanced a bit? Maybe background so we can distinguish it properly?

@Ashparshp
Copy link
Contributor

@upsaurav12 Thanks for your contribution, let's discuss this on the website's call. Please add this as an agenda item to the meeting minutes.

Let's have some feedback about it!!

@upsaurav12
Copy link
Author

upsaurav12 commented Aug 4, 2024

@upsaurav12, can the subitem be enhanced a bit? Maybe background so we can distinguish it properly?

@Ashparshp Yes i am working on it. I am planning to add background property like meshery.io navbar
image

@l5io
Copy link
Contributor

l5io commented Aug 9, 2024

🚀 Preview for commit 83dfca3 at: https://66b60f7e46bbbc68f37c7e59--layer5.netlify.app

@l5io
Copy link
Contributor

l5io commented Aug 9, 2024

🚀 Preview for commit 278d077 at: https://66b624992d2931773ee40f3b--layer5.netlify.app

@upsaurav12
Copy link
Author

@vishalvivekm @Ashparshp i have removed + sign and made background color toggle with dark mode/light mode.
i am working on improvement of sub-item like background .

@Ashparshp
Copy link
Contributor

Thank you, @upsaurav12. Feel free to discuss and ask anything if needed. Please provide an update as we discussed in the last call about the color.

@upsaurav12
Copy link
Author

@Ashparshp will do

Signed-off-by: upsaurav12 . <[email protected]>
@l5io
Copy link
Contributor

l5io commented Aug 17, 2024

🚀 Preview for commit 02c9461 at: https://66c0650d9a4e0d4be1aa67c5--layer5.netlify.app

@upsaurav12
Copy link
Author

Hi @Ashparshp @Muhammed-Moinuddin I have done work of most of the feedback but still confused about the background color of sub-item container
i have added color btw.
i have also made playground as a secondary button make them of full width.

Signed-off-by: upsaurav12 . <[email protected]>
@l5io
Copy link
Contributor

l5io commented Aug 19, 2024

🚀 Preview for commit deaf3ce at: https://66c311a4d0dc708e64d12b92--layer5.netlify.app

Signed-off-by: upsaurav12 . <[email protected]>
@l5io
Copy link
Contributor

l5io commented Aug 19, 2024

🚀 Preview for commit cabf6cb at: https://66c31f0fb5731ca519cdbfbf--layer5.netlify.app

@Muhammed-Moinuddin
Copy link
Contributor

Great work @upsaurav12 !
Overall it looks good. For sub-item container background choose these colors: https://layer5.io/static/context-visuals-1-87dde2e31da6ac24398052c332e5ff89.png
I've checked them they look great. code-90 and 10 of accent grey.

@upsaurav12
Copy link
Author

upsaurav12 commented Aug 20, 2024

Great work @upsaurav12 ! Overall it looks good. For sub-item container background choose these colors: https://layer5.io/static/context-visuals-1-87dde2e31da6ac24398052c332e5ff89.png I've checked them they look great. code-90 and 10 of accent grey.

@Muhammed-Moinuddin Thank for suggestion will work on that and push the changes

Signed-off-by: upsaurav12 . <[email protected]>
@l5io
Copy link
Contributor

l5io commented Aug 20, 2024

🚀 Preview for commit afee72d at: https://66c4d11a3a2f752159df2ced--layer5.netlify.app

@upsaurav12
Copy link
Author

upsaurav12 commented Aug 20, 2024

@Muhammed-Moinuddin can you check my newest commit. I have made changes for background color

@Muhammed-Moinuddin
Copy link
Contributor

@Muhammed-Moinuddin can you check my newest commit. I have made changes for background color

Maybe you missed my point, you have not picked the right colors that I recommended. Here you can find them: https://layer5.io/projects/sistent/identity/color/code. Accent-grey code-10 for dark and Accent-grey code-90 for light mode.

@upsaurav12
Copy link
Author

Sorry for my mistake , i was not able to find the hex-code for the color you mentioned. just added approx
Thank you for your help

@upsaurav12
Copy link
Author

upsaurav12 commented Aug 23, 2024

@Muhammed-Moinuddin it is now looking like this should i commit the changes

cinnamon-2024-08-26T1549320530.mp4

@upsaurav12
Copy link
Author

upsaurav12 commented Aug 26, 2024

Sorry @amitamrutiya @Muhammed-Moinuddin Video was not uploaded properly .
I have updated the video file can you review it ??

@l5io
Copy link
Contributor

l5io commented Aug 26, 2024

🚀 Preview for commit eb79465 at: https://66cc5a8be850fcd1f74464fd--layer5.netlify.app

@Ashparshp
Copy link
Contributor

@upsaurav12, What's the update?

@upsaurav12
Copy link
Author

@Ashparshp i have added background color for both dark and light mode as suggested by @Muhammed-Moinuddin

@Ashparshp
Copy link
Contributor

@upsaurav12, thank you.. Let's discuss this in our upcoming website meeting this Monday, if you're available. Could you please add this to the meeting minutes?

Signed-off-by: upsaurav12 . <[email protected]>
@l5io
Copy link
Contributor

l5io commented Sep 9, 2024

🚀 Preview for commit 833f546 at: https://66debe47b00b1b02a6d3207a--layer5.netlify.app

@upsaurav12
Copy link
Author

Hi @Ashparshp Sir , I was not active for couple of weeks . So was not able to work for this PR. Now i am working for this issue.

@Ashparshp
Copy link
Contributor

@upsaurav12, no problem at all! Thanks for the update. Feel free to take your time, and let me know if you need any help as you work on the issue.

@upsaurav12
Copy link
Author

Last time when i attended the meeting. Only sub-items color thing was remaining and color on hovering were left.

@upsaurav12
Copy link
Author

Hi @Muhammed-Moinuddin,
In my recent commit can you pls review the hover color in list-items. I am not sure about the color

@sudhanshutech
Copy link
Member

@upsaurav12 lets hold this for now until we have a good ux'er to give feedback for this . Do you got other issues to work? Lets get you assigned on other important items.

@sudhanshutech sudhanshutech added the pr/hold Do not merge this PR label Oct 14, 2024
@upsaurav12
Copy link
Author

Thanks @sudhanshutech ,
I am already assigned on the issue and working on the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core-styles pr/hold Do not merge this PR
Development

Successfully merging this pull request may close these issues.

Improvement of Navbar in Layer5 website for mobile version or smaller version
5 participants