Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 304
Offline sign-in and registration error messages#2940
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
Merged
micahscopes
merged 5 commits into
learningequality:unstable
from
micahscopes:offline-signin-and-registration-errorsJun 9, 2021
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
c2966e6
allow custom offline status text
micahscopes 70b021d
preliminary offline status and connection error messages during regis…
micahscopes efa7714
Disable signup finish button when offline
micahscopes 856dffe
Mock vuex state in signup tests
micahscopes 3f090d3
Tweak offline account creation message
micahscopes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
17 changes: 13 additions & 4 deletions
17 contentcuration/contentcuration/frontend/accounts/pages/Create.vue
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
31 changes: 27 additions & 4 deletions
31 contentcuration/contentcuration/frontend/accounts/pages/Main.vue
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
13 changes: 12 additions & 1 deletion
13 contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js
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
9 changes: 9 additions & 0 deletions
9 contentcuration/contentcuration/frontend/accounts/pages/__tests__/main.spec.js
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
10 changes: 7 additions & 3 deletions
10 contentcuration/contentcuration/frontend/shared/views/OfflineText.vue
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
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
Could this alert be moved on top, right after the main heading
Create an account, same as it is on the sign in page, right after the Kolibri Studio heading?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.
Maybe they need to restart the browser or the device to get the connection back, and there's nothing more annoying than having to fill in the same (long) form again... 😒
Uh oh!
There was an error while loading. Please reload this page.
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.
to echo some of Radina's thoughts above, I'd also prefer the alert moved after the 'create an account' heading and remove the offline icon indicator. The registration form on Studio is fairly long and there's a change the user might miss the offline indicator regardless if they're viewing the top or bottom portion of the screen.
Ideally I'd like to repurpose and tweak this behavior in Kolibri for these two offline cases:
Let me know if this is possible, @micahscopes and I'll mock something up for it!
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 agree with the proposed sticky snackbar, good strategy to make the alert visible independent of where the user is along the form.
Also, as I suggested, let's rephrase the string to
You seem to be offline. Please connect to the internet to create an account.to avoid introducing the the termregistration.Uh oh!
There was an error while loading. Please reload this page.
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 feedback @radinamatic and @khangmach.
When I first got into this, a snackbar seemed ideal, but using the actual snackbar components appeared to be tricky at the time, just because these login pages aren't part of the same Vue app as the rest of Studio.
That said, I'm glad to take another look at that. In the meantime, could simply moving the message up to the top be a reasonable starting point to get the basic functionality in for now, with the intention of iterating and switching to snackbars going forward?
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.
@micahscopes yep we can do that in the meantime.
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.
👍🏽 from me too, just correct the strings so we can have one thing less to think about!