-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix occasional input freezing on pinned tabs #667
Conversation
.eraseToAnyPublisher() | ||
} | ||
.sink { [weak self] in | ||
.switchToLatest() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flatMap
is replaced with map
+ switchToLatest
(which is an equivalent of RxSwift's flatMapLatest
, otherwise unavailable in Combine).
The idea here is to project a stream of tabContent
events into a stream of webViewDid*
streams, but only handle the last result stream. Adding this significantly reduces the number of unnecessary calls to sink
.
To test it, you can remove switchToLatest
and replace map
with flatMap
, and put a print statement inside the sink
closure. Then observe 9-10 print outputs in the console when loading Grafana, while there are 4 calls when switchToLatest
is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm impressed at how clean this fix ended up being, nice work. I tested this on production and then on this build, it's perfect. 💯
# By Dominik Kapusta (2) and others # Via GitHub * develop: Fix issues with printing websites (#668) Fix focus bug with pinned tabs (#667) Drag window when dragging the only tab in a window (#665) Update Cookie Consent animations (#655) Add tests for config urls (#664) Recently Visited and History Groupings (#654) # Conflicts: # DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift # DuckDuckGo/Menus/MainMenu.storyboard # DuckDuckGo/Menus/MainMenu.swift # DuckDuckGo/Statistics/PixelEvent.swift
Task/Issue URL: https://app.asana.com/0/0/1202668155654614/f
Description:
TL;DR: This patch removes unnecessary calls to
showTabContent
.The fix for while background flashing introduced delaying a call to
showTabContent
until any of
webViewDidCommit
,webViewDidFail
orwebViewDidReceiveChallenge
is called(with this code specifically).
This was done to ensure that tab content is only updated when the website actually starts loading
(or fails or requires authentication).
Only one of these events is needed to switch to the new tab content, so the merge publisher
should be limited to a single event. It appears that when navigating to websites behind DDG SSO,
such as Asana,
webViewDidReceiveChallenge
is called also afterwebViewDidCommit
(sometimes multiple times), which seems to occasionally be breaking keyboard input.
Steps to test this PR:
The scenario is reproducible with Grafana dashboard search:
https://grafana.duckduckgo.com/d/TDyfl_UWk/duckduckgo-home?orgId=1&search=open&query=
In the current release app the input won't work until you click a mouse in the text field.
Testing checklist:
Internal references:
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM