Ranchero-Software / NetNewsWire

RSS reader for macOS and iOS.
https://netnewswire.com/
MIT License
8.37k stars 531 forks source link

Dark Mode: Flash of Undarkened Content #455

Open danielpunkass opened 6 years ago

danielpunkass commented 6 years ago

I noticed if you are clicking into a new feed, the Web View for the article content clears to white before reloading with the expected dark gray backdrop.

  1. Open NetNewsWire in Dark Mode.
  2. Click on any feed that is not currently selected, so that the detail view shows "No selection".
  3. Click on any article in that feed.

Result: the detail view flashes bright white before loading the expected dark-themed background.

This can probably be remedied by some combination of pre-loading the web view with theme-suitable content, or maybe making the web view have a transparent background?

danielpunkass commented 6 years ago

A faster way to test the flash of white is simly to cmd-click the selected news item to unselect and reselect. The white background appears for each transition.

danielpunkass commented 6 years ago

I confirmed with the View Debugger that the white is being drawn by NetNewsWire.DefaultWebView.

danielpunkass commented 6 years ago

It's not trivial to address this problem, but here is a summary:

  1. WKWebView seems to default to a white background while loading content for the first time, or when switching back to "empty" content.
  2. We could alleviate the problem by hiding the WKWebView when first loading it, and then re-showing it again when it's done loading. At this point any future loads should inherit the previous style of the web view until the new content gets loaded to supersede it.
  3. In the Slack, Matt Meissner mentioned a private SPI in WebKit:

webview.setValue(true, forKey: "useSystemAppearance")

Which apparently will achieve the desired use of correct background colors. We probably don't want to rely upon private SPI though.

I think a reasonable solution might be to simply "prime" the WebView with suitable empty content to match the current appearance. We might not even need to hide the webview in that case because the process of priming it when the view is loaded might complete before it's really substantially used.

danielpunkass commented 6 years ago

Update from Matt Meissner that the "useSystemAppearance" hack doesn't completely solve the flash on its own. I think looking into some clever combination of hiding and/or pre-loading the webview with pertinent content would be a reasonable approach.

vincode-io commented 6 years ago

I think a reasonable solution might be to simply "prime" the WebView with suitable empty content to match the current appearance. We might not even need to hide the webview in that case because the process of priming it when the view is loaded might complete before it's really substantially used.

I think this is most likely approach to getting the flashing issue resolved. We remove the NoSelectionView all together and use HTML to replace its functionality so that the WebView never hides again. Worse case is that we have a white flash when the application loads.

@brentsimmons If you approve this approach, I'll code it up and we can see if it works.

brentsimmons commented 6 years ago

@vincode-io I agree — sounds like a good approach. Approved.

stuartbreckenridge commented 6 months ago

This appears to have resurfaced quite prominently in iOS 17 in both flash-to-white & flash-to-black varieties.

From testing: any time your phone switches display mode you need to go through roughly three articles before the flashing behaviour stops.

See #4252

vincode-io commented 6 months ago

It looks like a little bit of code might got lost when we reverted to mac-release. The current webView should be removed from its super view before inserting the new one in WebViewController. I'm not 100% sure that is the problem, but that is the only thing I can see different from what used to work.

stuartbreckenridge commented 6 months ago

This is behaviour on the prod release. It's not a dev build.

vincode-io commented 6 months ago

In that case it might already be fixed on main since the implementation of how we mitigate the flashing has changed there.

@brentsimmons You will still want to take a look at my previous comment. It will cause a memory leak at the least if not addressed.