Show file download/upload/print affordances in app context#15050
Show file download/upload/print affordances in app context#15050rtibblesbot wants to merge 1 commit into
Conversation
npm Package VersionsWarning The following packages have changed files but no version bump:
If these changes affect published code, consider bumping the version. |
Build Artifacts
Smoke test screenshot |
rtibbles
left a comment
There was a problem hiding this comment.
A little bit more cleanup, but the code here looks broadly sensible.
| @@ -85,8 +78,7 @@ | |||
| computed: { | |||
| ...mapState('classSummary', ['learnerMap']), | |||
| exportDisabled() { | |||
There was a problem hiding this comment.
I think we could go a step further and instead just use "disableExport" prop anywhere "exportDisabled" is used now, and cut out the indirection?
There was a problem hiding this comment.
Good call — done in 01a946c. Removed the exportDisabled pass-through computed and used disableExport directly in the template v-if.
| }, | ||
| canDownload() { | ||
| return !this.isAppContext && this.downloadableFiles.length; | ||
| return this.downloadableFiles.length; |
There was a problem hiding this comment.
Done in 01a946c — canDownload now returns this.downloadableFiles.length > 0.
rtibbles
left a comment
There was a problem hiding this comment.
Just one more bit of cleanup, then please tidy up the commit history.
| }); | ||
|
|
||
| it('does not render if isAppContext is true', () => { | ||
| it('renders in app context when there are renderable document files', () => { |
There was a problem hiding this comment.
I think we can just remove this test - it was previously asserting this because of a special case, which no longer exists.
There was a problem hiding this comment.
Done — removed in 73c49d2. With the isAppContext gate gone, canDownload no longer special-cases app context, so this test was just a duplicate of "should render if isAppContext is false and there are renderable document files".
01a946c to
963dc96
Compare
rtibbles
left a comment
There was a problem hiding this comment.
Code makes sense - needs commit history tidy up, ready for manual QA though.
963dc96 to
c4918c6
Compare
|
Tidied the commit history — squashed the four per-component commits into a single commit ( |
Several file upload/download/print UI affordances were gated behind isAppContext, hiding them in the installed apps. That was a workaround for the assumption that apps could not handle file downloads; the Windows and Mac apps handle this natively and the Android WebView work is tracked in kolibri-installer-android#267. Remove the gates so the affordances render regardless of context: - DownloadButton "Save to device" button visibility no longer checks isAppContext. - ReportsControls coach report CSV export drops isAppContext from exportDisabled (and the stale "disable in app mode" comment). - DataPage facility CSV import/export and session/summary log download sections drop the canUploadDownloadFiles gate. - AllPasswordsPage "Print all passwords" button always renders and the title row always uses the two-column layout, on both the coach and facility passwords pages. Behaviour in non-app context is unchanged. Blocking Jest tests are removed and replaced with app-context render assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
c4918c6 to
972e8a8
Compare
Summary
isAppContextgates hid file upload/download/print affordances in installed apps — a workaround from when apps couldn't handle downloads.References
Fixes #14654. Android WebView download support: learningequality/kolibri-installer-android#267.
Reviewer guidance
Confirm each of:
On each of:
AI usage
Used Claude Code to remove the gates and update tests following a pre-approved plan. Verified with the affected Jest suites and prek lint.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
🟡 Waiting for feedback
Last updated: 2026-07-24 03:27 UTC