Merged
Conversation
Contributor
|
with this approach, it means we will never display the error alert. Even if the token expires amid the browsing of provider directories., the view will just switch to the Auth view, without giving a reason why. Is that what you intended in this PR? |
Contributor
Author
|
Yes, after consideration, I think it’s better for now than confusing most users with an error message on their first experience with Uppy+Instagram, and also blocking the Connect button with Informer message, for example now on Transloadit website:
Later after 1.0 we could improve by only showing the error when browsing, not when authenticating for the first time, as would be ideal, if that’s even possible. |
ifedapoolarewaju
approved these changes
Apr 24, 2019
ifedapoolarewaju
approved these changes
Apr 24, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I wondered if there’s a simple solution, and this seems to work! So maybe it’s less complex than we anticipated, or am I missing something @ifedapoolarewaju? Auth error informer is not shown, while other errors are.
#1425