Skip to content

Conversation

linjunpop
Copy link
Contributor

Highlight the current target, make it more visible.

The highlight means to change the left border color to #ddd464, no animation.

Example

If the URL target at #report/1, the function will be highlighted.

image

@sourcelevel-bot
Copy link

Hello, @linjunpop! This is your first Pull Request that will be reviewed by Ebert, an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its review page. Please check our documentation for more information.

@josevalim
Copy link
Member

Thanks @linjunpop! We need to wait for approval from @dignifiedquire but in my humble opinion we do fine without highlighting, it may even look confusing at first.

@dignifiedquire
Copy link
Contributor

Hmmm, I agree with @josevalim, if you take a look at how links inside a page are usually handled on most pages, is that they are not differently styled just because they are active. I fear this would confuse users, especially with something like changing the color to yellow which is considered a color of warning for many.

@linjunpop
Copy link
Contributor Author

@dignifiedquire I agree with the static color change is not a good idea. My initial idea is to do something like https://stackoverflow.com/questions/44535895/how-to-know-incoming-call-have-video-or-not-in-pjsip-in-android/44696972#44696972 (Just some random post, content is irrelevant ), but that requires to do some animation, I'm afraid that's too much, so I initial a changes on the color. The point is to visually guide me to the function when I click on the sidebar, maybe some better design decision? Or you think this improvement is useless, I will close this PR. 😸

@dignifiedquire
Copy link
Contributor

I see. Maybe we could blink, or permanently show the anchor link in front we use to grab it?

@linjunpop
Copy link
Contributor Author

How about blinking the background:

2017-06-28 07_11_28

@whatyouhide
Copy link
Member

For reference, Dash.app already does exactly this: it blinks once when clicking on a function. I think it works great.

ezgif com-video-to-gif

@linjunpop
Copy link
Contributor Author

Had change to blink like Dash's.

@josevalim josevalim merged commit 534140f into elixir-lang:master Jul 30, 2017
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

@linjunpop linjunpop deleted the highlight-target branch July 30, 2017 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants