-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
SF-3124 Add sync message before draft queued message #2923
Conversation
I said testing is not required, but feel free to undo that |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2923 +/- ##
=======================================
Coverage 80.90% 80.90%
=======================================
Files 533 533
Lines 31204 31205 +1
Branches 5060 5060
=======================================
+ Hits 25246 25247 +1
Misses 5210 5210
Partials 748 748 ☔ View full report in Codecov by Sentry. |
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.
Nice job Joseph! One small comment/suggestion.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @josephmyers)
src/SIL.XForge.Scripture/ClientApp/src/app/translate/draft-generation/draft-generation.component.html
line 280 at r1 (raw file):
@if (isSyncing()) { <h3>{{ t("draft_syncing") }}</h3> <p>{{ t("draft_syncing_detail") }}</p>
We could show the app-sync-progress bar here instead of the detail message and gears.
Code quote:
<p>{{ t("draft_syncing_detail") }}</p>
src/SIL.XForge.Scripture/ClientApp/src/assets/i18n/non_checking_en.json
line 191 at r1 (raw file):
"draft_is_ready": "Your draft is ready", "draft_syncing": "Draft initializing", "draft_syncing_detail": "Your project is being synced before queuing the draft.",
If you do decide to switch to the sync progress bar "draft_syncing" could say something like "Syncing projects".
Code quote:
"draft_syncing": "Draft initializing",
"draft_syncing_detail": "Your project is being synced before queuing the draft.",
958a85f
to
25150da
Compare
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.
Hey, this is a really interesting idea. Way to think outside the box! I've been considering this for a while today and looking over what it would mean for the UX, and I think we should forego this, for the following reasons:
- The gears serve as an open-ended (critically), introductory animation that communicates the setup state quite well.
- If we instead start everything off with a progress bar, they will associate that bar with the draft, whether or not we have text that says otherwise, just because they clicked the Start button and now they see a progress bar.
- Further, it would be a little strange to go from a progress bar, to the gears, then to a completely different progress bar.
I do appreciate the brainstorming, but I think we should pass, though I am quite open to your thoughts on this. We do have other, bigger ways we could change things, e.g. integrating all three phases into the circle progress bar, but I don't see a ton of value for the time it would cost.
Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @kylebuss)
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.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @josephmyers)
25150da
to
e42dbbb
Compare
e42dbbb
to
0e0c570
Compare
To do this, we check the queue count in the project doc and display that message if the build job state is queued. Once the sync count is zero, we display the regular queue message.
0e0c570
to
65a356d
Compare
To do this, we check the queue count in the project doc and display that message if the build job state is queued. Once the sync count is zero, we display the regular queue message.
This change is