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

feat(sidebar): highlight selected route #1330

Closed
wants to merge 4 commits into from
Closed

Conversation

setchy
Copy link
Member

@setchy setchy commented Jul 4, 2024

Highlight sidebar button when route being shown

@setchy setchy added the enhancement New feature or enhancement to existing functionality label Jul 4, 2024
@setchy setchy added this to the Release 5.10.0 milestone Jul 4, 2024
@bmulholland
Copy link
Collaborator

I expected to love it, but found the video confusing. I think it's because only two icons change color? Also, something about the red throws me off, like it's an error state.

That could just be because I'm not the one controlling it, though. So may or may not be valid feedback :)

Copy link
Collaborator

@bmulholland bmulholland left a comment

Choose a reason for hiding this comment

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

Code looks good :)

@setchy
Copy link
Member Author

setchy commented Jul 8, 2024

I expected to love it, but found the video confusing. I think it's because only two icons change color? Also, something about the red throws me off, like it's an error state.

That could just be because I'm not the one controlling it, though. So may or may not be valid feedback :)

It's meant to be orange, but agree it does come across differently. Is there a different color you think would work well?

@afonsojramos
Copy link
Member

Maybe no color, but a little triangle next to it? Let me draw something up!

@afonsojramos
Copy link
Member

Like this?
image

Otherwise, I'd just stick with the color on hover.

@setchy
Copy link
Member Author

setchy commented Jul 8, 2024

Or using a dot (ignore formatting/alignment for now) with matching icon color

Screenshot 2024-07-08 at 11 10 32 AM Screenshot 2024-07-08 at 11 10 37 AM

@setchy setchy removed this from the Release 5.10.0 milestone Jul 8, 2024
@setchy setchy requested a review from bmulholland July 8, 2024 20:25
@afonsojramos
Copy link
Member

afonsojramos commented Jul 8, 2024

image

This is too packed. And shifting the icon's position feels weird UX too.
I say let's go with the hover's opacity, simpler implementation and cleaner UI

@bmulholland
Copy link
Collaborator

Yeah I agree that moving the icon would be weird.

@setchy
Copy link
Member Author

setchy commented Jul 9, 2024

agree, didn't intend for the icons to move around in the final version, hence the (ignore formatting/alignment for now) comment (it was a quick/dirty prototype).

Nonetheless, will look to update the hover opacity instead

@setchy
Copy link
Member Author

setchy commented Jul 9, 2024

Me again 😅

How about a simple tailwind ring @afonsojramos @bmulholland ?

Screenshot 2024-07-09 at 9 24 27 AM Screenshot 2024-07-09 at 9 24 34 AM

I played around with different opacity combinations but found it not very useful.

@bmulholland
Copy link
Collaborator

I like the ring. Personal opinion though.

@setchy
Copy link
Member Author

setchy commented Jul 14, 2024

Updated to use a shadow. imho, this is the best of the options explored above.

Screen.Recording.2024-07-14.at.8.34.11.AM.mov

@setchy
Copy link
Member Author

setchy commented Jul 16, 2024

Moving back to draft as @afonsojramos is looking into some things on his side

@setchy setchy marked this pull request as draft July 16, 2024 17:15
@setchy
Copy link
Member Author

setchy commented Aug 28, 2024

Closing this concept for now

@setchy setchy closed this Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants