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

Add template code highlighting #1643

Merged
merged 2 commits into from
Nov 15, 2024
Merged

Add template code highlighting #1643

merged 2 commits into from
Nov 15, 2024

Conversation

mikaelGusse
Copy link
Collaborator

This PR adds the ability for the user interface to highlight code fragments that have been ignored in the comparison. The ignored parts are highlighted with a faint grey color to separate from the blue that is used for matching fragments. Feel free to suggest any improvements to the implementation or functionality!

Copy link
Member

@rien rien left a comment

Choose a reason for hiding this comment

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

Code is looking good. However, I'm not convinced of the color of the highlight currently:

Current situation rgba(0, 0, 0, 0.05)

image

Since it is completely black with only 5% opacity, it looks blue-ish somehow - which is very similar to the match highlight color. One could mistakenly think this is just a "faint" match.

Suggested change rgba(220, 220, 220, 1)

By making the ignored highlight color opaque, it is better distinguishable from the background and also "overwrites" the match highlight color if it overlaps.

image

However, in both situations I don't know if it is clear for users who do not know this feature whether we are highlighting ignored code or not. @bmesuere what is your opinion on this?

@mikaelGusse
Copy link
Collaborator Author

I agree that this new color is certainly clearer. I was also wondering if there should be some "legend" which could explain what the different colors mean. It could be behind a question mark on the page somewhere perhaps? Or behind some hoverable element? Not sure about that

@rien
Copy link
Member

rien commented Nov 15, 2024

I have been thinking about this issue, but cannot find a good satisfying solution.

To avoid stalling this PR I will approve & merge it and create a new issue to discuss these options further.

@rien rien merged commit 1d13eb5 into main Nov 15, 2024
26 checks passed
@rien rien deleted the template-code-highlight branch November 15, 2024 13:46
@bmesuere
Copy link
Member

@rien sorry, I missed the mention somehow. Since the code is ignored, I would also try to (also) "fade" the code itself instead of only the background. Note that we will probably need a similar solution if/when we ignore comments.

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.

3 participants