Fix/tutorial help missing translations#344
Fix/tutorial help missing translations#344siddharthvaddem merged 1 commit intosiddharthvaddem:mainfrom
Conversation
📝 WalkthroughWalkthroughRestructured tutorial localization strings in English, Spanish, and Simplified Chinese: replaced four explanation keys with five segmented keys and split Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ATTRIBUTION.md`:
- Around line 1-14: ATTRIBUTION.md currently acknowledges Recordly but omits the
required AGPL-3.0 license text and copyright notice and OpenScreen remains
MIT-licensed which conflicts with imported AGPL code; either (A) add the full
AGPL-3.0 license text and Recordly copyright notice to ATTRIBUTION.md, update
the project LICENSE to AGPL-3.0 (and any package metadata), and document the
provenance for the ported functions (mathUtils.ts: easeOutScreenStudio,
cubicBezier, clamp01; zoomRegionUtils.ts) so compliance is explicit, or (B)
remove/replace the AGPL-licensed ported code (those functions and related
zoom/pan logic) with a clean-room implementation and revert ATTRIBUTION.md to a
permissive notice—pick one approach and apply it consistently across
ATTRIBUTION.md, LICENSE, and source metadata.
In `@src/i18n/locales/zh-CN/dialogs.json`:
- Around line 30-34: The issue is hardcoded leading spaces inside the emphasized
spans in TutorialHelp.tsx which rely on trailing spaces in some locales but
break Chinese; remove the hardcoded space before the span contents so the JSX
uses "<span>{t('tutorial.remove')}</span>" and
"<span>{t('tutorial.covered')}</span>" (i.e., eliminate the leading space
characters around t("tutorial.remove") and t("tutorial.covered")) or, if you
prefer locale-side fixes, add trailing spaces to the Chinese strings "remove"
and "covered" in the zh-CN dialogs.json to match English/Spanish—prefer removing
the JSX spaces for a language-agnostic fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f26273ea-1010-4370-af55-da583fe00805
📒 Files selected for processing (7)
ATTRIBUTION.mdREADME.mdsrc/components/video-editor/videoPlayback/mathUtils.tssrc/components/video-editor/videoPlayback/zoomRegionUtils.tssrc/i18n/locales/en/dialogs.jsonsrc/i18n/locales/es/dialogs.jsonsrc/i18n/locales/zh-CN/dialogs.json
| # Attribution | ||
|
|
||
| OpenScreen thanks the following open-source projects for inspiring and contributing code: | ||
|
|
||
| ## Zoom & Pan Animation | ||
|
|
||
| The zoom and pan animation system in OpenScreen was inspired by and ported from [Recordly](https://github.com/Recordly-dev/Recordly). | ||
|
|
||
| The following functions in `src/components/video-editor/videoPlayback/` were ported from Recordly: | ||
|
|
||
| - `mathUtils.ts` — `easeOutScreenStudio()`, `cubicBezier()`, and `clamp01()` | ||
| - `zoomRegionUtils.ts` — zoom region strength, chaining, and connected pan transition logic | ||
|
|
||
| We apologize for the omission and thank Recordly's maintainers for their excellent work. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect local attribution content.
sed -n '1,200p' ATTRIBUTION.md
# Fetch upstream Recordly license for compliance comparison.
curl -fsSL https://raw.githubusercontent.com/Recordly-dev/Recordly/main/LICENSE | sed -n '1,200p'Repository: siddharthvaddem/openscreen
Length of output: 761
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try common LICENSE file names and paths on Recordly repo
echo "=== Attempting alternative LICENSE file locations ==="
for branch in main master develop; do
for filename in LICENSE LICENSE.md LICENSE.txt COPYING COPYING.md; do
url="https://raw.githubusercontent.com/Recordly-dev/Recordly/$branch/$filename"
echo "Trying: $url"
if curl -fsSL "$url" 2>/dev/null | head -1; then
echo "Found at: $url"
curl -fsSL "$url" | head -50
break 2
fi
done
done
echo "=== Check if Recordly repo still exists ==="
curl -fsSL https://api.github.com/repos/Recordly-dev/Recordly | head -20Repository: siddharthvaddem/openscreen
Length of output: 2297
🌐 Web query:
Recordly open source project license type GitHub
💡 Result:
The Recordly open source project, hosted at https://github.com/webadderall/Recordly, is an open-source screen recorder for Mac/Windows/Linux. Its license type is AGPL 3.0 (GNU Affero General Public License v3.0), as explicitly stated in the repository's documentation: "Recordly is licensed under the AGPL 3.0". The project website at https://recordly.dev/ also confirms it is free and open-source. Note that GitHub metadata shows "Other (NOASSERTION)", which typically indicates the license file needs manual verification, but the README provides the clear statement.
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check OpenScreen's license files
echo "=== Checking OpenScreen's LICENSE files ==="
ls -la LICENSE* COPYING* 2>/dev/null || echo "No standard LICENSE found in root"
# Check for LICENSES directory (common pattern for multi-licensed projects)
if [ -d "LICENSES" ]; then
echo "=== LICENSES directory exists ==="
ls -la LICENSES/
fi
# Check if package.json or any config declares licenses
echo "=== Checking for license declarations ==="
find . -maxdepth 2 -name "package.json" -o -name "LICENSE" -o -name "licenses.json" | head -20
# Inspect the src directory structure to understand ported code location
echo "=== Checking ported code location ==="
ls -la src/components/video-editor/videoPlayback/ 2>/dev/null || echo "Directory not found or no files"Repository: siddharthvaddem/openscreen
Length of output: 1000
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect OpenScreen's LICENSE
echo "=== OpenScreen LICENSE content ==="
cat -n LICENSE
# Check if mathUtils.ts or zoomRegionUtils.ts have license headers
echo ""
echo "=== Checking mathUtils.ts for license headers ==="
head -30 src/components/video-editor/videoPlayback/mathUtils.ts
echo ""
echo "=== Checking zoomRegionUtils.ts for license headers ==="
head -30 src/components/video-editor/videoPlayback/zoomRegionUtils.tsRepository: siddharthvaddem/openscreen
Length of output: 3305
Add AGPL 3.0 license text and copyright notice to ATTRIBUTION.md and update OpenScreen's project license accordingly.
Recordly is licensed under AGPL 3.0, a copyleft license that requires preservation of license text, copyright notices, and source code availability. The current ATTRIBUTION.md acknowledges the source but lacks the required AGPL 3.0 text and copyright notice. Additionally, OpenScreen is licensed under MIT, which is incompatible with AGPL 3.0 code. You must either:
- Include the full AGPL 3.0 license text and Recordly's copyright notice in ATTRIBUTION.md, and relicense OpenScreen under AGPL 3.0 or a compatible license; or
- Remove the AGPL 3.0 ported code and replace it with alternative implementations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ATTRIBUTION.md` around lines 1 - 14, ATTRIBUTION.md currently acknowledges
Recordly but omits the required AGPL-3.0 license text and copyright notice and
OpenScreen remains MIT-licensed which conflicts with imported AGPL code; either
(A) add the full AGPL-3.0 license text and Recordly copyright notice to
ATTRIBUTION.md, update the project LICENSE to AGPL-3.0 (and any package
metadata), and document the provenance for the ported functions (mathUtils.ts:
easeOutScreenStudio, cubicBezier, clamp01; zoomRegionUtils.ts) so compliance is
explicit, or (B) remove/replace the AGPL-licensed ported code (those functions
and related zoom/pan logic) with a clean-room implementation and revert
ATTRIBUTION.md to a permissive notice—pick one approach and apply it
consistently across ATTRIBUTION.md, LICENSE, and source metadata.
|
can you remove unrelated changes? |
| // Ported from Recordly (https://github.com/Recordly-dev/Recordly). | ||
| // See ATTRIBUTION.md for details. | ||
|
|
| // Ported from Recordly (https://github.com/Recordly-dev/Recordly). | ||
| // See ATTRIBUTION.md for details. | ||
|
|
| ## Attribution | ||
|
|
||
| Third-party attributions are listed in [ATTRIBUTION.md](./ATTRIBUTION.md). | ||
|
|
c6f55cc to
942fb17
Compare
The main branch has already applied the same tutorial.help key restructuring with slightly different intermediate values. Adopting main's version to resolve merge conflicts.
942fb17 to
485c95b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/i18n/locales/es/dialogs.json`:
- Around line 30-34: The localized string concatenation is missing a space
between tutorial.covered and tutorial.explanationAfter, causing "cubiertapor" to
render; fix the value for the "explanationAfter" key in
src/i18n/locales/es/dialogs.json (or adjust TutorialHelp.tsx) by ensuring a
leading space is present in the "explanationAfter" text (e.g., change "por un
segmento..." to " por un segmento...") so that tutorial.covered +
tutorial.explanationAfter renders correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6aac71ac-c9a5-4be8-8455-a3c685cee13e
📒 Files selected for processing (3)
src/i18n/locales/en/dialogs.jsonsrc/i18n/locales/es/dialogs.jsonsrc/i18n/locales/zh-CN/dialogs.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/i18n/locales/en/dialogs.json
- src/i18n/locales/zh-CN/dialogs.json
| "explanationBefore": "La herramienta de recorte funciona definiendo los segmentos que deseas", | ||
| "remove": "eliminar", | ||
| "explanationMiddle": " — cualquier parte", | ||
| "covered": "cubierta", | ||
| "explanationAfter": "por un segmento rojo será eliminada al exportar.", |
There was a problem hiding this comment.
Add the missing space before por — this renders kinda cursed.
In TutorialHelp.tsx, tutorial.covered and tutorial.explanationAfter are concatenated directly, so this currently renders as cubiertapor un segmento....
🌙 Tiny localization fix
- "explanationAfter": "por un segmento rojo será eliminada al exportar.",
+ "explanationAfter": " por un segmento rojo será eliminada al exportar.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/i18n/locales/es/dialogs.json` around lines 30 - 34, The localized string
concatenation is missing a space between tutorial.covered and
tutorial.explanationAfter, causing "cubiertapor" to render; fix the value for
the "explanationAfter" key in src/i18n/locales/es/dialogs.json (or adjust
TutorialHelp.tsx) by ensuring a leading space is present in the
"explanationAfter" text (e.g., change "por un segmento..." to " por un
segmento...") so that tutorial.covered + tutorial.explanationAfter renders
correctly.
Pull Request Template
Description
Motivation
Type of Change
Related Issue(s)
Screenshots / Video
Screenshot (if applicable):
Video (if applicable):
Testing
Checklist
Thank you for contributing!
Summary by CodeRabbit