Skip to content

Conversation

@m7moud
Copy link

@m7moud m7moud commented Sep 15, 2025

Please test this case:
The links for citations and sources in the Arabic locale are something like: arhttps://usul.ai/t/nadhra-al-naeem-fi-makarem-akhlaq-al-rasool-al-kareem/5186

It should be: https://usul.ai/ar/t/nadhra-al-naeem-fi-makarem-akhlaq-al-rasool-al-kareem/5186

Summary by CodeRabbit

  • Bug Fixes
    • Share links now include the locale immediately after the domain (e.g., https://example.com/en/...), ensuring the correct language context is preserved when sharing book reader pages.
    • Improves consistency of shared URLs across locales and prevents issues caused by incorrect locale placement.
    • Existing page paths, optional version parameters, and selection fragments are preserved, so previously shared content remains accurately referenced.

@vercel
Copy link
Contributor

vercel bot commented Sep 15, 2025

@m7moud is attempting to deploy a commit to the DigitalSeem's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 15, 2025

Walkthrough

Adjusted locale handling in useBookShareUrl.getUrl: locale segment is now appended to the base URL with a trailing slash instead of being prefixed before the origin. Other URL components (reader path, optional versionId query, selection fragment) remain unchanged. No exported/public signatures were modified.

Changes

Cohort / File(s) Summary
Share URL locale suffix change
src/lib/share.ts
In getUrl, changed locale augmentation from prefixing the base URL (url = pathLocale + url) to suffixing it (url = \${url}${pathLocale}/``). Other URL construction steps unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop on links from root to tail,
A suffix trail, no longer preface veil.
With slashes neat and locale snug,
The share URL gets a tidy hug.
Tap-tap paws on the path I tread—
To /en/ I bounce, then onward I spread.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Fix the url localization for the share links" is concise, focused, and accurately describes the primary change (correcting locale placement in share URLs) as shown in src/lib/share.ts and the PR objectives about Arabic link formatting, so it meaningfully communicates the main fix to reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

🧹 Nitpick comments (3)
src/lib/share.ts (3)

26-28: Bug fixed; normalize path joining to avoid // and locale/path edge cases

The change resolves the arhttps://... bug. To make joins robust regardless of trailing/leading slashes and whether pageReader returns /t/... or t/..., normalize both sides.

Apply this diff:

-    if (locale !== routing.defaultLocale) url = `${url}${pathLocale}/`;
-    url += navigation.books.pageReader(slug, pageIndex + 1);
+    if (locale !== routing.defaultLocale)
+      url = `${url.replace(/\/+$/, "")}${pathLocale.replace(/^\/?/, "/").replace(/\/+$/, "")}`;
+    url = `${url.replace(/\/+$/, "")}/${navigation.books.pageReader(slug, pageIndex + 1).replace(/^\/+/, "")}`;

29-29: Encode query param value

Safer in case versionId contains special characters.

Apply this diff:

-    if (versionId) url += `?versionId=${versionId}`;
+    if (versionId) url += `?versionId=${encodeURIComponent(versionId)}`;

50-54: Handle clipboard write failures

Avoid showing success when navigator.clipboard is unavailable or denied.

Apply this diff:

-    const url = getUrl({ slug, pageIndex, versionId, selection });
-    await navigator.clipboard.writeText(url);
-
-    toast.success(t("reader.chat.copied"));
+    const url = getUrl({ slug, pageIndex, versionId, selection });
+    try {
+      await navigator.clipboard.writeText(url);
+      toast.success(t("reader.chat.copied"));
+    } catch {
+      toast.error("Failed to copy URL");
+    }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df87097 and 318059c.

📒 Files selected for processing (1)
  • src/lib/share.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/share.ts (1)
src/i18n/config.ts (1)
  • routing (4-42)
🔇 Additional comments (1)
src/lib/share.ts (1)

24-24: Ensure client-only usage or guard window access

Hook accesses window.location.origin and navigator.clipboard — ensure it's only invoked from client components (add "use client" to callers) or add a runtime guard (e.g., check typeof window !== "undefined" and navigator?.clipboard before use).

Location: src/lib/share.ts — let url = window.location.origin;

Repo scan did not return call sites; verify callers are client components or add guards.

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.

1 participant