Closed swsnr closed 1 year ago
I don't really understand the CI failure… did I do anything wrong?
Regarding the CI failures: no. Just disregard them.
Unfortunately you made the changes on master branch which is not the one used for development, only for releases. It would be great if you could make another PR started from develop branch. Otherwise I can manually include your changes in develop.
So far the code looks good, only thing to "criticise" would be process.waitFor. You should use a timeout there to prevent a permanently hung application there.
No new PR required: I've rebased my changes onto develop
and changed the base branch of this PR accordingly.
That said, develop
also doesn't seem to use the detected dark mode to adjust the MediathekView user interface 🤔 Is this a planned feature? Would you accept a corresponding pull request (include a setting like "Follow system dark mode")?
Currently it is manual only as not all interface elements support dark mode. I would accept another PR for that feature but be aware that almost all settings dialog components are built with JFormDesigner and the interoperability of the design files and java code must be working after changes.
Detects Gnome Dark mode by reading the corresponding interface setting from Gnome.
Notes
I also drive-by-fixed a use of a deprecated overload of Runtime.exec in the mac OS implementation. I hope that's fine, but if you like I can also remove that commit again.
The subprocess stuff in isMacOsDarkMode felt overly complicated to me, so I used a shorter and simpler approach in the Gnome implementation, but I'm only superficially familiar with Java so I may have overlooked potential issues in my approach. If you like I can also change the code to look like the macOS stuff.
When testing I noticed that the method is called, and the result is printed on stdout, but the interface stays light regardless of whether dark mode is enabled or not… which I found a bit disappointing; I had hoped mediathekview would follow dark mode automatically. What's the reason for detecting dark mode but then not following it? Just not implemented so far? If so, is it planned?