Skip to content

Fix/tutorial help missing translations#344

Merged
siddharthvaddem merged 1 commit intosiddharthvaddem:mainfrom
ekkoitac:fix/tutorial-help-missing-translations
Apr 18, 2026
Merged

Fix/tutorial help missing translations#344
siddharthvaddem merged 1 commit intosiddharthvaddem:mainfrom
ekkoitac:fix/tutorial-help-missing-translations

Conversation

@ekkoitac
Copy link
Copy Markdown
Contributor

@ekkoitac ekkoitac commented Apr 5, 2026

Pull Request Template

Description

Motivation

Type of Change

  • New Feature
  • Bug Fix
  • Refactor / Code Cleanup
  • Documentation Update
  • Other (please specify)

Related Issue(s)

Screenshots / Video

Screenshot (if applicable):

![Screenshot Description](path/to/screenshot.png)

Video (if applicable):

<video src="path/to/video.mp4" controls width="600"></video>

Testing

Checklist

  • I have performed a self-review of my code.
  • I have added any necessary screenshots or videos.
  • I have linked related issue(s) and updated the changelog if applicable.

Thank you for contributing!

Summary by CodeRabbit

  • Documentation
    • Restructured tutorial instruction text in English, Spanish, and Chinese so step copy is now composed from smaller segments—improves placement of the keyboard cue, spacing/wording around that cue, and gender/article agreement across translations for clearer, more natural guidance.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 5, 2026

📝 Walkthrough

Walkthrough

Restructured tutorial localization strings in English, Spanish, and Simplified Chinese: replaced four explanation keys with five segmented keys and split step1Description into step1DescriptionBefore and step1DescriptionAfter to enable in-line composition of keyboard/CTA fragments.

Changes

Cohort / File(s) Summary
Tutorial localization — English
src/i18n/locales/en/dialogs.json
Replaced tutorial.explanation / explanationRemove / explanationCovered / explanationEnd with explanationBefore / remove / explanationMiddle / covered / explanationAfter. Split step1Description into step1DescriptionBefore and step1DescriptionAfter.
Tutorial localization — Spanish
src/i18n/locales/es/dialogs.json
Same key reshaping as English: removed old explanation parts and added segmented keys (explanationBefore, remove, explanationMiddle, covered, explanationAfter) and split step1Description into before/after parts; grammar adjusted for Spanish.
Tutorial localization — Simplified Chinese
src/i18n/locales/zh-CN/dialogs.json
Mirrors the English/Spanish refactor: old explanation keys removed, new segmented keys added, and step1Description split into step1DescriptionBefore / step1DescriptionAfter.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • FabLrc

Poem

keys split like careful scissors, lowkey tidy ✂️
fragments stitched where sentences used to be
en / es / zh hum in sync — kinda cursed, kinda neat
translations ready, ready to sing on cue 🎶

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning PR description is mostly template boilerplate with no concrete details about the changes, motivation, or testing steps filled in. Fill in Description and Motivation sections explaining the translation refactor. Add concrete testing steps and clarify if this is actually a bug fix (not just a documentation update).
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately describes the main change: refactoring tutorial localization keys across multiple language files to fix missing translation compositions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between da16872 and c6f55cc.

📒 Files selected for processing (7)
  • ATTRIBUTION.md
  • README.md
  • src/components/video-editor/videoPlayback/mathUtils.ts
  • src/components/video-editor/videoPlayback/zoomRegionUtils.ts
  • src/i18n/locales/en/dialogs.json
  • src/i18n/locales/es/dialogs.json
  • src/i18n/locales/zh-CN/dialogs.json

Comment thread ATTRIBUTION.md Outdated
Comment on lines +1 to +14
# 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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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 -20

Repository: 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.ts

Repository: 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:

  1. 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
  2. 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.

Comment thread src/i18n/locales/zh-CN/dialogs.json Outdated
@siddharthvaddem
Copy link
Copy Markdown
Owner

can you remove unrelated changes?

Comment on lines +1 to +3
// Ported from Recordly (https://github.com/Recordly-dev/Recordly).
// See ATTRIBUTION.md for details.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like this

Comment on lines +1 to +3
// Ported from Recordly (https://github.com/Recordly-dev/Recordly).
// See ATTRIBUTION.md for details.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this/

Comment thread README.md Outdated
Comment on lines +95 to +98
## Attribution

Third-party attributions are listed in [ATTRIBUTION.md](./ATTRIBUTION.md).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.

@ekkoitac ekkoitac force-pushed the fix/tutorial-help-missing-translations branch from c6f55cc to 942fb17 Compare April 18, 2026 00:48
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.
@ekkoitac ekkoitac force-pushed the fix/tutorial-help-missing-translations branch from 942fb17 to 485c95b Compare April 18, 2026 00:52
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 942fb17 and 485c95b.

📒 Files selected for processing (3)
  • src/i18n/locales/en/dialogs.json
  • src/i18n/locales/es/dialogs.json
  • src/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

Comment on lines +30 to +34
"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.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

@siddharthvaddem siddharthvaddem merged commit 974fde4 into siddharthvaddem:main Apr 18, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants