-
Notifications
You must be signed in to change notification settings - Fork 167
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: multiple fast navigate calls #20446
base: main
Are you sure you want to change the base?
Conversation
Fix issue where a slow connection and fast `navigate` calls throws exception due to faulty blocker state change. Fixes #20404
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.
The patch looks good.
Smoke tested it, couldn't spot anything weird.
However, I couldn't reproduce the original issue.
Another pair of eyes would be beneficial.
Easiest way to replicate original issue is to have a hilla layout and 2 flow layouts, setting the network throttling to slow 4g , then navigating to a flow view from a flow view through the layout menu and clicking 2 times on the menu item. |
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.
We talked with @platosha on Friday, this patch slightly changes how navigation works: with a slow network, when you first time click on the menu link and immediately click on the second one, the application will ignore the second navigation and basically all other navigations until the first one completes.
Maybe it's better to queue these navigations and apply the latest in the end. Flow navigation works like this IIRC.
@platosha promised to see how we can do it.
@caalador do you think it's better to queue like I described?
We could push the navigate to The issue is that when the programmer uses react-router |
Quality Gate passedIssues Measures |
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.
Thanks for the tip, the exception has eventually appeared for me and after the latest changes I don't see it anymore and the multiple navigations are queued and handled as I expected - the last click wins in the end.
I think we can try adding a test that simulates the same steps, I wonder if these codes works:
public class UseBlockerIT extends ChromeDeviceTest {
@Test
public void test() {
ChromeDriver driver = (ChromeDriver) getDriver();
DevTools devTools = driver.getDevTools();
devTools.send(Network.enable(Optional.empty(), Optional.empty(), Optional.empty()));
devTools.send(Network.emulateNetworkConditions(
false, // offline
100, // latency (ms)
750000, // download through (bits per second)
250000, // upload throughput (bits per second)
Optional.of(ConnectionType.CELLULAR4G), // connection type
Optional.empty(),
Optional.empty(),
Optional.empty()
));
// open a page
// click twice
// check for errors
// check the latest navigation
}
}
If Flow repo is not a suitable place for it, could we add it to platform test module?
Fix issue where a slow connection
and fast
navigate
calls throwsexception due to faulty blocker
state change.
Fixes #20404