Make audio-input respect readonly #1939
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds a readonly prop to AudioInput to gate UI controls. Forwards readonly from multi-ws and rich-multi-ws inputs. Updates locales to include the new "No audio" string. No non-UI logic changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
the
💻 FW Lite
UI unit Tests 1 files ± 0 40 suites +37 23s ⏱️ +23s Results for commit 5f6a003. ± Comparison against base commit c20ed56. This pull request removes 10 and adds 82 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
frontend/viewer/src/locales/fr.po (1)
666-669: Translate "No audio" in French to avoid fallback to English.Provide a localized msgstr to keep UX consistent with other translated audio strings.
Apply this diff:
msgid "No audio" -msgstr "" +msgstr "Aucun audio"frontend/viewer/src/locales/es.po (1)
666-669: Add Spanish translation for "No audio".Untranslated strings will render in English; suggest a concise placeholder.
msgid "No audio" -msgstr "" +msgstr "Sin audio"frontend/viewer/src/locales/ko.po (1)
666-669: Add Korean translation for "No audio".Fill in msgstr to match surrounding audio labels.
msgid "No audio" -msgstr "" +msgstr "오디오 없음"frontend/viewer/src/locales/id.po (1)
666-669: Add Indonesian translation for "No audio".Prevents fallback to English in readonly placeholder.
msgid "No audio" -msgstr "" +msgstr "Tidak ada audio"frontend/viewer/src/lib/components/field-editors/audio-input.svelte (1)
258-266: Nice RO gating and UX copyHiding “Add audio” in RO and showing a localized “No audio” placeholder matches expectations. i18n key usage via
$tlooks correct.For minor a11y improvement, consider announcing this state change to screen readers when it toggles:
- <div class="text-muted-foreground p-1"> + <div class="text-muted-foreground p-1" role="status" aria-live="polite"> {$t`No audio`} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
frontend/viewer/src/lib/components/field-editors/audio-input.svelte(3 hunks)frontend/viewer/src/lib/components/field-editors/multi-ws-input.svelte(1 hunks)frontend/viewer/src/lib/components/field-editors/rich-multi-ws-input.svelte(1 hunks)frontend/viewer/src/locales/en.po(1 hunks)frontend/viewer/src/locales/es.po(1 hunks)frontend/viewer/src/locales/fr.po(1 hunks)frontend/viewer/src/locales/id.po(1 hunks)frontend/viewer/src/locales/ko.po(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-02T14:27:02.745Z
Learnt from: myieye
PR: sillsdev/languageforge-lexbox#1720
File: frontend/viewer/src/locales/es.json:1786-1790
Timestamp: 2025-06-02T14:27:02.745Z
Learning: Spanish locale file (frontend/viewer/src/locales/es.json) contains generated code that may have null line numbers in origin entries due to limitations in the code generation process.
Applied to files:
frontend/viewer/src/locales/es.po
📚 Learning: 2025-05-27T06:18:33.852Z
Learnt from: hahn-kev
PR: sillsdev/languageforge-lexbox#1710
File: frontend/viewer/src/project/browse/BrowseView.svelte:17-19
Timestamp: 2025-05-27T06:18:33.852Z
Learning: The NewEntryButton component in frontend/viewer/src/project/NewEntryButton.svelte already internally checks features.write permission and conditionally renders based on write access, so external disabled props are not needed.
Applied to files:
frontend/viewer/src/lib/components/field-editors/audio-input.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build UI / publish-ui
- GitHub Check: Build API / publish-api
- GitHub Check: check-and-lint
- GitHub Check: frontend-component-unit-tests
- GitHub Check: Build FW Lite and run tests
- GitHub Check: frontend
🔇 Additional comments (4)
frontend/viewer/src/lib/components/field-editors/audio-input.svelte (2)
305-321: Edit actions correctly hidden in read-onlyConditionally rendering the actions menu only when
!readonlyprevents replace/remove in RO while still allowing playback/seek. This is the right tradeoff for “read-only”.
61-67: Audit complete:readonlyprop correctly forwarded in all<AudioInput>instances
The newreadonlyprop (defaulting tofalse) maintains backward compatibility and matches other editors’ patterns. Verified that every<AudioInput>usage forwards{readonly}:
- frontend/viewer/src/lib/components/field-editors/rich-multi-ws-input.svelte (lines 78–81)
- frontend/viewer/src/lib/components/field-editors/multi-ws-input.svelte (lines 53–56)
No missing forwards detected—approving these changes.
frontend/viewer/src/lib/components/field-editors/rich-multi-ws-input.svelte (1)
78-81: Readonly is forwarded correctly to AudioInputForwarding
{readonly}alongside the existing binder keeps behavior consistent with other editors. No behavioral regressions expected.frontend/viewer/src/lib/components/field-editors/multi-ws-input.svelte (1)
53-56: Readonly forwarding added and looks goodPassing
{readonly}toAudioInputbrings the audio branch in line with the text inputs’ RO behavior. Binder and onchange wiring are unchanged.
frontend/viewer/src/lib/components/field-editors/audio-input.svelte
Outdated
Show resolved
Hide resolved
|
This seemed like a nice place to insert this small tweak. It needs more work but it's there now. |
2 participants
Before:
After:
