search

ReadAlongs / Studio-Web

Suite of web packages for creating interactive ReadAlongs
https://readalong-studio.mothertongues.org/
Other
11 stars 9 forks source link

fix: cannot download other formats #351

Closed deltork closed 1 month ago

deltork commented 1 month ago

PR Goal?

bug fix

Fixes?

347

Feedback sought?

sanity check

Priority?

high

Tests added?

to be added to studio test suites

How to test?

use the pr-preview editor to upload an offline format and try to export non web formats

Confidence?

Version change?

bug fix

This PR depends on the merging of PR https://github.com/ReadAlongs/Studio/pull/247

semanticdiff-com[bot] commented 1 month ago

Review changes with SemanticDiff.

Analyzed 1 of 1 files.

Overall, the semantic diff is 41% smaller than the GitHub diff.

Filename Status
:heavy_check_mark: packages/studio-web/src/app/editor/editor.component.ts 40.86% smaller
github-actions[bot] commented 1 month ago

PR Preview Action v1.4.8 :---: Preview removed because the pull request was closed. 2024-10-11 15:35 UTC

joanise commented 1 month ago

This looks safe to merge already, even before we merge https://github.com/ReadAlongs/Studio/pull/247 I'm not sure why it's labelled draft, it seems to work correctly already. Given it would fix the live deployment, I'm tempted to merge it ASAP, @roedoejet if you have a chance to review it too maybe we could do that soon, even while Del is away.

roedoejet commented 1 month ago

This looks safe to merge already, even before we merge ReadAlongs/Studio#247 I'm not sure why it's labelled draft, it seems to work correctly already. Given it would fix the live deployment, I'm tempted to merge it ASAP, @roedoejet if you have a chance to review it too maybe we could do that soon, even while Del is away.

I don't understand parts of this PR, but I unfortunately don't have enough time to dig into it either. Del is away for the weekend? If you're confident in these changes, go for it, @joanise otherwise my in-depth review would have to wait until next week

joanise commented 1 month ago

I don't understand parts of this PR, but I unfortunately don't have enough time to dig into it either. Del is away for the weekend? If you're confident in these changes, go for it, @joanise otherwise my in-depth review would have to wait until next week

Fair enough. I guess I'll test more, to make sure it really does work as I think it does, and decide what to do based on that.