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

Windows GUI: Dark mode for HTML view #857

Merged
merged 1 commit into from
May 31, 2024

Conversation

cjee21
Copy link
Collaborator

@cjee21 cjee21 commented May 31, 2024

Inject CSS style into the HTML to give it a dark mode.

Screenshot 2024-05-31 171823

Let me know if you want to make changes to how it looks.

Inject CSS style into the HTML to give it a dark mode.
@JeromeMartinez
Copy link
Member

Great!
The scroll bar is still not in dark mode, any hope that it can be in dark mode too?

@cjee21
Copy link
Collaborator Author

cjee21 commented May 31, 2024

Great! The scroll bar is still not in dark mode, any hope that it can be in dark mode too?

No idea for that one. This application is still using the old IE engine for HTML which isn't aware of dark mode as far as I know. There is a newer TEdgeBrowser for displaying HTML using the Edge engine but probably would not work on older Windows.

@JeromeMartinez
Copy link
Member

There is a newer TEdgeBrowser for displaying HTML using the Edge engine but probably would not work on older Windows.

We dropped WinXP and our minimal requirement is Win7 now so I think it should be fine, if you are motivated go ahead on the switch.
If you don't want to risk to spend too much time on that, you may create a branch with a proof of concept only and we'll test it on a Win7 VM.

@cjee21
Copy link
Collaborator Author

cjee21 commented May 31, 2024

There is a newer TEdgeBrowser for displaying HTML using the Edge engine but probably would not work on older Windows.

We dropped WinXP and our minimal requirement is Win7 now so I think it should be fine, if you are motivated go ahead on the switch. If you don't want to risk to spend too much time on that, you may create a branch with a proof of concept only and we'll test it on a Win7 VM.

Windows 8.1 and below is dropped for this according to https://learn.microsoft.com/en-us/microsoft-edge/webview2/.
Also will require users to have Microsoft Edge WebView2 installed on their system if it is not already auto-installed.

@JeromeMartinez
Copy link
Member

Also will require users to have Microsoft Edge WebView2 installed on their system if it is not already auto-installed.

Win7 is still used by few % of people despite it is EOL for a long time and it would not worth it to drop this support yet or bother people with an additional install, but if you don't mind we would like to use your skills when you are here, it would be great to have a patch ready and when we consider that it is time to switch we just merge it. We just don't promise about the time line.
And maybe we could update your PR by dynamically use one of the 2 engines depending on the OS, detection at runtime.

@JeromeMartinez JeromeMartinez merged commit 7800432 into MediaArea:master May 31, 2024
3 checks passed
@JeromeMartinez
Copy link
Member

(Note that this PR won't be in the next release as we already launched the build process, it will be for the one after the next one)

@cjee21
Copy link
Collaborator Author

cjee21 commented May 31, 2024

Win7 is still used by few % of people despite it is EOL for a long time and it would not worth it to drop this support yet or bother people with an additional install, but if you don't mind we would like to use your skills when you are here, it would be great to have a patch ready and when we consider that it is time to switch we just merge it. We just don't promise about the time line. And maybe we could update your PR by dynamically use one of the 2 engines depending on the OS, detection at runtime.

Are there plans to use the Qt version for Windows? If so maybe better to spend effort on improving the Qt one instead. The latest Qt has native dark mode built-in for Windows without needing to do anything (looks and works better than this VCL implementation). By the way, the Windows 11 context menu if implemented can be used for any MediaInfo version on Windows. It is not dependent on this VCL version so it is fine to spend effort on that even if there are plans to retire the VCL version in the future.

@cjee21 cjee21 deleted the html-darkmode branch May 31, 2024 10:29
@JeromeMartinez
Copy link
Member

Are there plans to use the Qt version for Windows?

True

It is not dependent on this VCL version so it is fine to spend effort on that even if there are plans to retire the VCL version in the future.

True too.

We are torn between polishing the VCL and switching to Qt at some point, but the gap between VCL and what is currently in Qt version is huge and we always postpone the full development.

@JeromeMartinez
Copy link
Member

It is in 24.05.1.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jun 2, 2024

If you don't want to risk to spend too much time on that, you may create a branch with a proof of concept only and we'll test it on a Win7 VM.

@JeromeMartinez I wasn't going to do this if it required significant code changes but it didn't so here it is: https://github.com/cjee21/MediaInfo/tree/edge-webview2

It should use Edge WebView2 engine when available else fallback to IE engine. I only tested on Windows 11.
Screenshot 2024-06-02 201430
This is the right-click menu when using Edge engine.

#853 (comment) is fixed by this, at least when using the Edge engine. The scrollbar is still not dark mode. I guess this requires some API (maybe this?) to set the current scheme but no idea how.

If it works on Windows 7 and 10 as well and you are satisfied with it, then I can make the pull request.

EDIT: Forgot to mention, need to do the steps under Installing the Edge WebView2 package via GetIt at https://docwiki.embarcadero.com/RADStudio/Sydney/en/Using_TEdgeBrowser_Component_and_Changes_to_the_TWebBrowser_Component to get the Edge WebView2 engine to work.
WebView2Loader.dll needs to be distributed along with MediaInfo.exe.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jun 3, 2024

Edit: Deleting temporary HTML file is now in main and no longer part of this patch

I noticed MediaInfo left the temp HTML file on exit so I added code to delete that file on exit.

Note: We probably can use data URL and not need the temp file in the future when no longer need to support old browser engines.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URLs#browser_compatibility
Maybe not due to length limitations.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jun 3, 2024

TODO: Test if WebView2 UDF will auto change to AppData and not crash when MediaInfo is installed in the read-only Program Files.
Updated code to just set WebView2 UDF to MediaInfo's "AppData" directory (the same folder as the Plugin folder) to be safe. The documentation doesn't seem to be reliable as EdgeUserDataFolder property didn't exist so used env to set instead.
https://learn.microsoft.com/en-us/microsoft-edge/webview2/concepts/user-data-folder?tabs=win32
https://docwiki.embarcadero.com/Libraries/Alexandria/en/SHDocVw.TWebBrowser.EdgeUserDataFolder

@cjee21
Copy link
Collaborator Author

cjee21 commented Jun 6, 2024

We are torn between polishing the VCL and switching to Qt at some point, but the gap between VCL and what is currently in Qt version is huge and we always postpone the full development.

@JeromeMartinez Tried building Qt version using latest Qt (6.7.1) and latest MSVC (2022) just to see how it looks.

Here's how it looks on Windows 11:
Screenshot 2024-06-06 215704
Screenshot 2024-06-06 215732

Changes I made to Qt version for testing are available here: https://github.com/cjee21/MediaInfo/tree/Qt

@cjee21 cjee21 mentioned this pull request Jun 17, 2024
@JeromeMartinez
Copy link
Member

If it works on Windows 7 and 10 as well and you are satisfied with it, then I can make the pull request.

Please do open a PR, easier to track.

@cjee21
Copy link
Collaborator Author

cjee21 commented Jun 18, 2024

We are torn between polishing the VCL and switching to Qt at some point, but the gap between VCL and what is currently in Qt version is huge and we always postpone the full development.

@JeromeMartinez I made some changes to the Qt version while testing. The changes are available here: https://github.com/cjee21/MediaInfo/tree/Qt. If you would like me to make a PR for this as well, just let me know.

This is how my branch currently looks on Windows 11 and Ubuntu:
Screenshot 2024-06-19 012642
Screenshot 2024-06-19 012718
Screenshot from 2024-06-19 01-35-38
Screenshot from 2024-06-19 01-35-56

@JeromeMartinez
Copy link
Member

@JeromeMartinez I made some changes to the Qt version while testing. The changes are available here: https://github.com/cjee21/MediaInfo/tree/Qt. If you would like me to make a PR for this as well, just let me know.

We are also interested in updates on the Qt version so PR please, thank you!

@cjee21 cjee21 mentioned this pull request Jun 19, 2024
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.

2 participants