Skip to content

Dark Mode support in HTMLTextView #148

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

Merged
merged 7 commits into from
Sep 29, 2022
Merged

Dark Mode support in HTMLTextView #148

merged 7 commits into from
Sep 29, 2022

Conversation

mhdostal
Copy link
Member

This adds dark mode support through CSS elements in the HTML for the TextPopupElement and the HTML view. It also changes the color of the round rect border from .black to .primary, in order to catch light/dark mode changes.

Light Mode:

image

Dark Mode:

image

@mhdostal mhdostal marked this pull request as ready for review September 19, 2022 21:41
@mhdostal mhdostal requested a review from dfeinzimer September 19, 2022 21:41
@dfeinzimer
Copy link
Collaborator

Everything looks good to me other than the 1 small suggestion above.

@mhdostal mhdostal requested a review from dfeinzimer September 19, 2022 22:47
dfeinzimer
dfeinzimer previously approved these changes Sep 19, 2022
Copy link
Collaborator

@dfeinzimer dfeinzimer left a comment

Choose a reason for hiding this comment

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

Looks good!

dfeinzimer
dfeinzimer previously approved these changes Sep 21, 2022
Copy link
Collaborator

@dfeinzimer dfeinzimer left a comment

Choose a reason for hiding this comment

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

Turns out black isn't black, but #1C1C1E, when in DarkMode

Nice find!

@mhdostal
Copy link
Member Author

@dfeinzimer Based on team feedback, I've removed the round rect border from the text element.

@dfeinzimer
Copy link
Collaborator

Latest changes look good.

This may be better under it's own issue/PR but I noticed that when switching from one popup to another (without closing the panel in-between), the newly selected popup fails to display.

I tried a quick sanity test (showing the latest identify point instead of PopupView) to see if this was an issue with Floating Panel but that worked okay.

Any idea?

2022-09-27 14 37 25

dfeinzimer
dfeinzimer previously approved these changes Sep 27, 2022
@mhdostal
Copy link
Member Author

This may be better under it's own issue/PR but I noticed that when switching from one popup to another (without closing the panel in-between), the newly selected popup fails to display.

@dfeinzimer Thanks. It's an issue with the example and has been reported already. I noticed it a few weeks ago, and fixed it, but I broke it again with recent changes to the example.

@mhdostal
Copy link
Member Author

On some devices, the Dark Mode background color is a dark, dark gray, not black. On other devices, it is true black. Need to dynamically insert the dark mode color into the HTML.

@mhdostal
Copy link
Member Author

On some devices, the Dark Mode background color is a dark, dark gray, not black. On other devices, it is true black. Need to dynamically insert the dark mode color into the HTML.

I'm seeing different background colors on the iPhone 13 mini simulator and the iPad Pro 9.7 simulator. I also see the difference in the "example" list (if you look closely, the background of the list doesn't match the background of the rest of the screen):

image

The solution is to make the background of the HTML view transparent. This way it will just expose the view background, whether dark or white. The foreground (text) color will remain black or white. This provides the following interesting effect, if you have a slightly (or greatly) transparent background for the floating panel:

image

@mhdostal
Copy link
Member Author

@dfeinzimer Ok, hopefully one last time.

@mhdostal mhdostal merged commit 9704bd6 into v.next Sep 29, 2022
@mhdostal mhdostal deleted the mhd/DarkMode_HTML branch September 29, 2022 15:27
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