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

new color generation for matches #1311

Merged
merged 8 commits into from
Nov 28, 2023
Merged

new color generation for matches #1311

merged 8 commits into from
Nov 28, 2023

Conversation

Kr0nox
Copy link
Member

@Kr0nox Kr0nox commented Sep 25, 2023

This PR changes the color generation for matches.
Instead of distributing the colors across the hue spectrum, now there is a set of 7 (handpicked) colors from which each match is colored.
The algortihm for choosing colors: First all matches get sorted 3 times. (By line number in first and second submission and by number of tokens). It then runs through the list of matches and chooses the first color it can find that is not neigboring this match in any of the three lists. To avoid having only the first few colors or even only two alternating colors, we dont start with the first color when having found a fitting color but go on with the next in the list and go through the list cyclical.

@Kr0nox
Copy link
Member Author

Kr0nox commented Sep 25, 2023

We should take a look at the list of colors in the next dev meeting

@tsaglam tsaglam added enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies labels Sep 26, 2023
@tsaglam tsaglam added this to the v5.0.0 milestone Oct 4, 2023
@Kr0nox
Copy link
Member Author

Kr0nox commented Oct 13, 2023

The currently chosen colors
grafik

@Kr0nox Kr0nox marked this pull request as ready for review October 13, 2023 16:18
@TwoOfTwelve TwoOfTwelve self-requested a review November 7, 2023 14:17
@sebinside
Copy link
Member

The currently chosen colors grafik

If you don't have a strong reason against it, I would opt for using the orange in the upper right corner instead of the red-like color in the second row, as it is still distinct but, well, less red.

Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

Works fine, the new colors look great with real data! I only have minor and easy to implement change requests, please see the comments above.

@Kr0nox
Copy link
Member Author

Kr0nox commented Nov 27, 2023

If you don't have a strong reason against it, I would opt for using the orange in the upper right corner instead of the red-like color in the second row, as it is still distinct but, well, less red.

I changed it. Could you have a quick look to make sure the two oranges are not too similar @sebinside

@Kr0nox Kr0nox requested a review from sebinside November 27, 2023 13:10
@sebinside
Copy link
Member

Oh, my mistake: I did not mean second row but second column. That's what happens if you review to many PRs at once, sorry! Please see the attached picture:
image

Copy link

[JPlag Report Viewer] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

The colors look great, thank you!

@Kr0nox Kr0nox merged commit 5b9acf3 into develop Nov 28, 2023
17 checks passed
@Kr0nox Kr0nox deleted the report-viewer/new-colorgen branch November 28, 2023 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change report-viewer PR / Issue deals (partly) with the report viewer and thus involves web-dev technologies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants