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

Disable connect button when connecting #85

Merged
merged 2 commits into from Jan 19, 2018

Conversation

4 participants
@skddc
Copy link
Member

skddc commented Jan 17, 2018

Disables the connect button both visually and functionally upon pressing it, until either a discovery error appears, or it redirects to the storage provider's OAuth dialog.

connected to #26

skddc added some commits Jan 17, 2018

Disable connect button when connecting
Disables the connect button both visually and functionally upon pressing
it, until either a discovery error appears, or it redirects to the
storage provider's OAuth dialog.
@gregkare

This comment has been minimized.

Copy link
Member

gregkare commented Jan 18, 2018

LGTM!

@skddc

This comment has been minimized.

Copy link
Member

skddc commented Jan 18, 2018

Can I have a second +1 plz? /cc @remotestorage/core

@michielbdejong

This comment has been minimized.

Copy link
Member

michielbdejong commented Jan 19, 2018

Looks like

this.rsConnectButton.disabled = false;
is the only place where the button is re-enabled? What if discovery was successful, and you then successfully disconnect again, wouldn't the Connect button stay disabled forever (i.e. until page refresh) then?

@skddc

This comment has been minimized.

Copy link
Member

skddc commented Jan 19, 2018

is the only place where the button is re-enabled? What if discovery was successful, and you then successfully disconnect again, wouldn't the Connect button stay disabled forever (i.e. until page refresh) then?

Good point! I suppose it would. Will check and fix if necessary.

@galfert

This comment has been minimized.

Copy link
Member

galfert commented Jan 19, 2018

Isn't that automatically taken care of by the redirect after successful discovery?

@skddc

This comment has been minimized.

Copy link
Member

skddc commented Jan 19, 2018

Wait, actually the discovery always involves a redirect. So the widget state is always reset.

Edit: lol, 1 second after you. :)

@galfert

This comment has been minimized.

Copy link
Member

galfert commented Jan 19, 2018

I just tested it. Works fine and changes also look ok to me.

👍

@michielbdejong

This comment has been minimized.

Copy link
Member

michielbdejong commented Jan 19, 2018

LGTM! :shipit:

@skddc

This comment has been minimized.

Copy link
Member

skddc commented Jan 19, 2018

Thanks, shipping it...

@skddc skddc merged commit 1ffead5 into master Jan 19, 2018

@skddc skddc deleted the feature/26-connect_button branch Jan 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment