[#449] - Improve coloring of tree maps

[fo-code]

In order to make the tree map easier to read, I changed the colors handling.
Now, the Jenkins colors from the Design Library are used and loaded dynamically for the table and the tree view.

Examples:

before:

after:

before:

after:

[codecov]

Codecov Report

Merging #455 (a57e861) into master (cad5c71) will increase coverage by 0.01%.
The diff coverage is 80.83%.

@@             Coverage Diff              @@
##             master     #455      +/-   ##
============================================
+ Coverage     72.78%   72.79%   +0.01%     
- Complexity     1014     1025      +11     
============================================
  Files            87       88       +1     
  Lines          3770     3816      +46     
  Branches        437      439       +2     
============================================
+ Hits           2744     2778      +34     
- Misses          878      891      +13     
+ Partials        148      147       -1     

Impacted Files	Coverage Δ
...rage/model/visualization/colorization/ColorId.java	100.00% <ø> (ø)
...ins/plugins/coverage/model/CoverageTableModel.java	9.16% <11.11%> (-0.15%)	⬇️
...kins/plugins/coverage/model/CoverageViewModel.java	57.65% <31.25%> (-4.48%)	⬇️
...ugins/coverage/model/ChangeCoverageTableModel.java	9.37% <33.33%> (ø)
...s/coverage/model/IndirectCoverageChangesTable.java	10.34% <33.33%> (ø)
...odel/visualization/colorization/ColorProvider.java	100.00% <100.00%> (ø)
...sualization/colorization/ColorProviderFactory.java	100.00% <100.00%> (ø)
...isualization/colorization/CoverageChangeLevel.java	100.00% <100.00%> (ø)
...alization/colorization/CoverageColorJenkinsId.java	100.00% <100.00%> (ø)
...sualization/colorization/CoverageColorPalette.java	100.00% <100.00%> (ø)
... and 7 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[uhafner]

Would it help to use white as text color?

[uhafner]

This breaks the visualization on smaller displays.

[uhafner]

Can we remove these unrelated changes? Then we can merge the color PR and create a new release.

[uhafner]

See above

[uhafner]

In total I think the greens look better, but the reds not.

[uhafner]

the tree map is shown on the whole screen now

The problem with that approach is, that you then need to scroll on small displays. I found no way yet to always use the whole visible screen only for the trees. Maybe the problem is that the side panel is too large.

[fo-code]

The problem with that approach is, that you then need to scroll on small displays. I found no way yet to always use the whole visible screen only for the trees. Maybe the problem is that the side panel is too large.

Okay, well I do not have a small screen and when I try it by resizing the window, it fills just the visible screen due to the resize event, but this might behave differently on a real small screen.
Nonetheless, I think in most cases h-100 is useful, even if a few users have to scroll.
I guess not that many developer use a small laptop screen all the time and even if it is the case, a bigger tree map allows that more file names are displays and the scrolling is limited.
What do you think about that?

In total I think the greens look better, but the reds not.

I totally agree - I tried some different approaches and found a better solution.
Also, I recognized that the package colors are generated by the echarts API, but these should be set dependend on the coverage.
Furthermore, the alpha of the currently used colors are problematic in the case of the map, so I removed these.
Therefore, I locally changed some values including the echarts plugin and the result can be shown above - I updated the "after" examples.
The new approach shows the coverage of each node exactly and different levels are clearly separable.

I am not available for two weeks now, so I would update the code afterwards - but maybe you can already give me some feedback on the new approach?

[uhafner]

The problem with that approach is, that you then need to scroll on small displays. I found no way yet to always use the whole visible screen only for the trees. Maybe the problem is that the side panel is too large.

Okay, well I do not have a small screen and when I try it by resizing the window, it fills just the visible screen due to the resize event, but this might behave differently on a real small screen.
Nonetheless, I think in most cases h-100 is useful, even if a few users have to scroll. I guess not that many developer use a small laptop screen all the time and even if it is the case, a bigger tree map allows that more file names are displays and the scrolling is limited. What do you think about that?

I would rather prefer to create different views for small and large screens. This shouldn't be too hard with Bootstraps classes. See https://github.com/jenkinsci/code-coverage-api-plugin/blob/master/plugin/src/main/resources/coverage/coverage-table.jelly#L14 for an example

(See https://getbootstrap.com/docs/5.2/layout/grid/ for a documentation of all responsive classes)

In total I think the greens look better, but the reds not.

I totally agree - I tried some different approaches and found a better solution. Also, I recognized that the package colors are generated by the echarts API, but these should be set dependend on the coverage. Furthermore, the alpha of the currently used colors are problematic in the case of the map, so I removed these. Therefore, I locally changed some values including the echarts plugin and the result can be shown above - I updated the "after" examples. The new approach shows the coverage of each node exactly and different levels are clearly separable.

I am not available for two weeks now, so I would update the code afterwards - but maybe you can already give me some feedback on the new approach?

The new colors look better now. I wonder if we should use the red, orange and green colors of Jenkins as well here?
https://weekly.ci.jenkins.io/design-library/Colors/

[fo-code]

I would rather prefer to create different views for small and large screens.

So you mean we can add h-100 for large screens only in order to use the full screen here but also support small screens without h-100?

I wonder if we should use the red, orange and green colors of Jenkins as well here?

Yes of course, so you mean to change the currently used colors in CoverageColorPalette and insert the RGB codes of the Jenkins colors manually? Or is there a possibility to load the library colors dynamically as for example RGB values in Java?

[uhafner]

I would rather prefer to create different views for small and large screens.

So you mean we can add h-100 for large screens only in order to use the full screen here but also support small screens without h-100?

Yes, that shouldn't be too hard to implement.

I wonder if we should use the red, orange and green colors of Jenkins as well here?

Yes of course, so you mean to change the currently used colors in CoverageColorPalette and insert the RGB codes of the Jenkins colors manually? Or is there a possibility to load the library colors dynamically as for example RGB values in Java?

Getting them dynamically should work for the charts if we change the proxy methods:

So rather than using

            viewProxy.getOverview(function (t) {
                createOverview(t.responseObject(), 'coverage-overview');
            });

we could use

            viewProxy.getOverview(colors, function (t) {
                createOverview(t.responseObject(), 'coverage-overview');
            });

and fill colors with a JSON object, that contains the important styles. Something like:

{
 green: '#00ff00',
 orange: '#00ff00',
 red: '#00ff00'
}

(See https://weekly.ci.jenkins.io/design-library/Colors/ for the available colors).

You can get the colors then via:

    function getTextColor() {
        return getComputedStyle(document.body).getPropertyValue('--text-color') || '#333';
    }

Then the colors automatically will be adapted to the selected theme.

[fo-code]

Yes, that shouldn't be too hard to implement.

I agree, this looks like a good solution.

Getting them dynamically should work for the charts if we change the proxy methods:

Thanks for the explanation.
As I understand this, the color JSON has to be set manually? In this case, I prefer using an Enum since this allows using these colors within the whole plugin and it is easier to integrate.
More specific what I wanted to know is if for example this color JSON can be requested or if I can call a Java API to request these color hex codes within the Java code of the coverage plugin? I am not really familiar of how the design library works...

[uhafner]

Yes, that shouldn't be too hard to implement.

I agree, this looks like a good solution.

Getting them dynamically should work for the charts if we change the proxy methods:

Thanks for the explanation. As I understand this, the color JSON has to be set manually? In this case, I prefer using an Enum since this allows using these colors within the whole plugin and it is easier to integrate. More specific what I wanted to know is if for example this color JSON can be requested or if I can call a Java API to request these color hex codes within the Java code of the coverage plugin? I am not really familiar of how the design library works...

It works the other was round: there is no way for the server to call the client (and there is no API on the client side). You need to wrap all information in an existing Ajax call from the client to the server:

In Jenkins processing is started on the client where the user selects the HTTP address. This is mapped on the server side into a jelly view. This jelly view is then rendered as HTML and send to the client that will render it. In this view we can invoke an associated JS file and call back to the server using Ajax. I.e., when the client currently starts a request to get the coverage data (the chart is empty in the jelly view) the client can provide an additional parameter (that is the reason that I am proposing a JSON object on the JS side, where we can put everything into what we want. I use the same thing in the trend chart configuration). This JSON can be interpreted on the server side to extract the colors. An enum will not work as it is constant on the server side, but actually each client might have its own colors. Maybe we can setup a call to discuss this part in more detail?

[fo-code]

Yes, this would be perfect! I sent you an email.

[fo-code]

@uhafner I finished the color rework.
The Jenkins colors from the Design Library are now in use and for the table and the tree map dynamically loaded.
The fallback colors correspond to the Jenkins colors aswell.
Currently, some HSL colors are provided in the dark mode - these cases are not handled yet and the fallback is used.
The size of the tree map is like before in order to work on all screen sizes.

[uhafner]

I like the new design! The only mandatory changes are the viewpoint adjustments which are unrelated to this PR and should be changed in a different PR.

[uhafner]

Can we remove these unrelated changes? Then we can merge the color PR and create a new release.

[fo-code]

Nice to hear and thanks for the review!
Then, everything should be ready to be released. :)