Fix POM Graph colors for dark themes

[johntor]

I noticed a problem using the Graph of a POM when a dark theme is selected.

The foreground color was requested from the Theme while backgrounds are hardcoded into the Class causing printing WHITE on Bright Colors.

With this fix I added transparency to these hardcoded colors so the become darker on darker backgrounds.

I also removed gradients that looked a bit outdated to my eyes.

[mbien]

hi @johntor, thanks for working on this.

it certainly does look much better and I agree the gradients have to go given that NetBeans is using a theme called Flat LAF by default :)

However, I still see problems with FlatLAF dark, what theme are you using since your screenshorts show white font on dark background.

(FlatLaf dark, items in graph don't have focus)
before PR:

after PR:

[mbien]

the scope markers are now also painted as solid blocks which is going to need some tweaks too:

(Cupertino Dark, scopes set to Test)

[johntor]

I did not notice all these! If you like the idea of the removed gradients we can optimize the code more.

[mbien]

@johntor would be great. Simply force push into this PR if you want to make changes.

I believe the font color might need to be hard coded if the custom (non-standard) background fill of the text element is active - otherwise there gonna be always some theme which will clash.

[johntor]

Yes I was thinking determining if the theme is light/dark then apply fixed black/white colors for fonts

[mbien]

Yes I was thinking determining if the theme is light/dark then apply fixed black/white colors for fonts

if the background is taken from the theme, its ok to take the font color from the theme too. But as soon the background is changed, for example when set to orange, the color can't be taken from the theme anymore - it has to fit to the orange background.

I wouldn't hardcode all font colors, only conditional when the background actually changes.

[neilcsmith-net]

Yes, two hard coded colours for each pallette option based on value of nb.dark.theme boolean from UIManager might be way to go, at least for a quick fix.

[mbien]

Yes, two hard coded colours for each pallette option based on value of nb.dark.theme boolean from UIManager might be way to go, at least for a quick fix.

i don't think any of this needs to know if the dark theme is active or not if it follows the simple rule of changing both, foreground and background at the same time but never only one of them while leaving the other to the theme.

[johntor]

Yes, two hard coded colours for each pallette option based on value of nb.dark.theme boolean from UIManager might be way to go, at least for a quick fix.

i don't think any of this needs to know if the dark theme is active or not if it follows the simple rule of changing both, foreground and background at the same time but never only one of them while leaving the other to the theme.

Then we have problems to apply your light-grey text over orange or yellow. I will go with the hardcoded solution and we see how it goes...

[mbien]

Then we have problems to apply your light-grey text over orange or yellow.

why would you do that? I am trying to say that if you change the background to orange, you have to change the text to a fitting color too, so that it is readable on orange. You can't leave the text color up to the theme and hope it will still work.

[johntor]

Hello again I think is fine now but how am I supposed to update the file and how I force my pull request?

[mbien]

Hello again I think is fine now but how am I supposed to update the file and how I force my pull request?

you have now two commits, I think the easiest option in this particular case is to remove the last commit again with:

git reset HEAD^

This will only remove the commit, the changes you made will remain in the source code.

Now you can simply commit with NetBeans again but check the Amend last Commit checkbox, this will add the changes to the first commit, instead of creating a second one again - basically updating the commit.

(the same command from the console would be git commit --amend --no-edit)

To replace the remote delivery branch with your local delivery branch you type in a console:

git push -f git@github.com:johntor/netbeans delivery:delivery

[neilcsmith-net]

i don't think any of this needs to know if the dark theme is active or not if it follows the simple rule of changing both, foreground and background at the same time but never only one of them while leaving the other to the theme.

You create a palette of background colours that works with black-ish text, and one that works with white-ish text. Working on something similar with the visual library myself at the moment.

However, this fix looks good for now. If we can get it amended and force pushed as above, we can merge later today for 17-rc3.

[mbien]

You create a palette of background colours that works with black-ish text, and one that works with white-ish text. Working on something similar with the visual library myself at the moment.

If the background would be theme aware - the foreground must be theme aware too, I agree.

But in this particular case the background fill colors (red/orange) already work with dark/light themes, additionally to that the boxes have an outline which is controlled by the theme - which makes it extra resistant to clash with theme colors. So the main problem here was that some boxes set the background fill without setting the font color. That is the reason I said I do not think the logic has to be aware of the theme at all - it only has to follow the rule to not pick the foreground from a theme color while hard coding the background.

The visual for "Scopes" will need more work since it used to be a gradient but this is outside the scope of this PR :) (I don't think it will look good in any color the way it is currently rendered as simple box in a box)

[johntor]

I think current solution is simple and efficient for any color theme. Keep in mind the new feature of changing the colors of Flat Laf.

This solution is adaptioning theme colors while keeping original colors with BLACK foreground to boxes with background yellow orange and red.

[mbien]

can you revert the formatting changes you did in the most recent commit We try to avoid unrelated changes in PRs which fix things.

[eirikbakke]

Thanks for this patch! I'm fine with this patch as long as it is tested well on both light and dark FlatLAF modes.

A general comment for future PRs is to try to keep the patches free of unnecessary formatting changes; this makes them easier to review. If you'd like to clean up formatting, it's good to do all the formatting changes in a single commit.

(A side note for @neilcsmith-net: aren't "release candidate" versions supposed to be very stable, with only critical bugfixes going into them? Patches like these benefit from being tested for a few weeks in dev environments before going into production.)

[mbien]

(A side note for @neilcsmith-net: aren't "release candidate" versions supposed to be very stable, with only critical bugfixes going into them? Patches like these benefit from being tested for a few weeks in dev environments before going into production.)

@eirikbakke well the colors and gradients were borderline unreadable before, so the bar is lower here for a hotfix. This is only a UI change which removes gradients and tweaks colors so it has low risk to cause trouble. I agree with the formatting changes which were added by the second commit - those should be reverted.

[eirikbakke]

@mbien Yeah, I trust your judgment here.

(I just remember having made "cosmetic-only" patches like this myself only to find I introduced a NullPointerException somewhere a week later...)

[johntor]

I am sorry that I have no time to completed this Pull Request till the end right now... (busy at work)
Can anybody else take care of it?
I promise next time I will complete the work all the way...
Thank you all!
Keep on your great job!

[neilcsmith-net]

A side note for @neilcsmith-net: aren't "release candidate" versions supposed to be very stable, with only critical bugfixes going into them?

Bug fixes until rc3, only critical fixes afterwards - https://lists.apache.org/thread/hyjbsz55zb9xfcnccghkrsvqsnt83nwf

Yes, there are risks to lower priority fixes, but it's worth it to improve the release experience IMO.

[mbien]

@johntor no worries. I can take over. Will remove the formatting changes and force push into this PR. Thanks for your work!

This might add me as co author to the commit. Lets see how this works.

[mbien]

squashed, reverted formatting and force pushed

[mbien]

@eirikbakke @neilcsmith-net I am willing to accept this one as my side quest, if it causes followup problems assign issues to me.

[neilcsmith-net]

Thanks @johntor and @mbien Will merge tomorrow.

@mbien given my comments on #5390 asking for it to be rebased, we can share any side quest 😆

[mbien]

@johntor congrats on your first contribution.

[johntor]

@johntor congrats on your first contribution.

Thank you very much! I thought contributing to a complicated project like Netbeans is too difficult... I realized that I was wrong!

I will be back!

Take care!