Fix/tutorial help missing translations#344
Fix/tutorial help missing translations#344ekkoitac wants to merge 2 commits intosiddharthvaddem:mainfrom
Conversation
- Update ATTRIBUTION.md to list clamp01() alongside easeOutScreenStudio() and cubicBezier() - Add inline attribution header comment to mathUtils.ts - Add inline attribution header comment to zoomRegionUtils.ts
Also remove humble self-deprecating line from README (closes siddharthvaddem#320)
📝 WalkthroughWalkthroughPR adds attribution documentation for Recordly-sourced components, updates README with attribution reference, adds source attribution comments to ported code files, and restructures i18n translation keys for tutorial text to enable segmented messaging around dynamic keyboard shortcut inputs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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.
| "explanationBefore": "剪辑工具通过定义您要", | ||
| "remove": "移除", | ||
| "explanationMiddle": "的片段来工作。任何被红色剪辑区域", | ||
| "covered": "覆盖", | ||
| "explanationAfter": "的部分将在导出时被剪掉。", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify hardcoded leading spaces around highlighted tutorial tokens.
fd -i 'TutorialHelp.tsx' --exec rg -n -C2 'tutorial\.remove|tutorial\.covered|font-bold"> '
# Verify segmented zh-CN strings currently don't include those spaces themselves.
fd -i 'dialogs.json' src/i18n/locales --exec rg -n -C1 'explanationBefore|remove|explanationMiddle|covered|explanationAfter'Repository: siddharthvaddem/openscreen
Length of output: 1979
Hardcoded spaces in JSX create unnatural spacing in Chinese text.
The TutorialHelp.tsx component uses hardcoded spaces before emphasized tokens:
<span className="text-[`#ef4444`] font-bold"> {t("tutorial.remove")}</span>
<span className="text-[`#ef4444`] font-bold"> {t("tutorial.covered")}</span>
English and Spanish locale strings include trailing spaces to accommodate this pattern, but the Chinese locale strings do not. This results in unnatural spacing in Chinese output (e.g., "您要 移除" instead of "您要移除").
Either remove the hardcoded JSX space or add trailing spaces to Chinese locale strings to match English/Spanish behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/i18n/locales/zh-CN/dialogs.json` around lines 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.
|
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). | ||
|
|
2 participants
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
Documentation
Localization