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

Fix an extra reloading when an error page is manually reloaded #574

Merged
merged 2 commits into from
Aug 24, 2017

Conversation

yuya-oc
Copy link
Contributor

@yuya-oc yuya-oc commented Aug 24, 2017

Before submitting, please confirm you've

Please provide the following information:

Summary
Fix an extra reloading when an error page is manually reloaded.

This PR cancels the automated reloading when an error page is manually reloaded by Ctrl+R.

Issue link
#573

Test Cases

  1. Unplug ethernet cable or disable Wi-Fi to get error page.
  2. Start the application.
  3. Make sure that the error is NOT ERR_INTERNET_DISCONNECTED (-106).
  4. Repair internet connection then press Ctrl+R.
  5. Extra reloading should not happen within 30 seconds.

Additional Notes
https://circleci.com/gh/yuya-oc/desktop/327#artifacts

@yuya-oc yuya-oc added this to the v3.8.0 milestone Aug 24, 2017
@jasonblais
Copy link
Contributor

Thanks @yuya-oc! I tried testing it but unfortunately get a ERR_INTERNET_DISCONNECTED (-106) error each time..

Are there additional steps involved to ensure I get the (-105) error instead?

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Aug 24, 2017

Unfortunately I'm not sure. My PC has -105 error because probably there are other virtual Ethernet for VirtualBox.

@S6066 Wondering if you would like to test the app? You can download the latest app from here. https://327-67276967-gh.circle-artifacts.com/0/tmp/circle-artifacts.exAz08v/mattermost-desktop-3.7.0-win64.zip

@Apjue
Copy link

Apjue commented Aug 24, 2017

@yuya-oc Tested, no automatic reload occured.
(Didn't understand the virtual ethernets problem, I have 2 VMWare "Network Adapters" and it still runs as expected)

@jasonblais
Copy link
Contributor

Maybe Jonathan have your help code review to familiarize yourself with the desktop app codebase?

I'll work on tweaking our review process to match something similar to platform (i.e. PM/test review, dev review, ready to merge)

@yuya-oc
Copy link
Contributor Author

yuya-oc commented Aug 24, 2017

Thanks @S6066. I'm also not sure why -105 error happens for me. But I'm glad to hear that the problem was fixed for you.

@yuya-oc yuya-oc merged commit e9e82c6 into mattermost:master Aug 24, 2017
@yuya-oc yuya-oc deleted the fix-unexpected-reloading branch August 24, 2017 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants