Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: page title is not updated after language switch. fixed: #4799 #4813

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

mynetfan
Copy link
Collaborator

@mynetfan mynetfan commented Nov 5, 2024

Description

修复切换语言时,页面标题没有自动更新的问题

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Checklist

ℹ️ Check all checkboxes - this will indicate that you have done everything in accordance with the rules in CONTRIBUTING.

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs:dev command.
  • Run the tests with pnpm test.
  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • New Features
    • Introduced dynamic title updates based on the current route and user preferences across multiple applications.
  • Bug Fixes
    • Removed outdated dynamic title setting logic from the router guard functions, ensuring consistent title management.
  • Chores
    • Updated import statements to include necessary modules for enhanced title functionality.

@mynetfan mynetfan requested review from anncwb, vince292007 and a team as code owners November 5, 2024 02:46
Copy link

changeset-bot bot commented Nov 5, 2024

⚠️ No Changeset found

Latest commit: dec342e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Nov 5, 2024

Walkthrough

The changes in this pull request primarily involve the addition of dynamic title management functionality across several bootstrap.ts files in different applications. The watchEffect function from Vue is introduced to monitor application preferences and update the document title based on the current route. Corresponding modifications in the guard.ts files involve the removal of previously existing dynamic title setting logic, ensuring that the control flow remains intact while eliminating outdated title management.

Changes

File Path Change Summary
apps/web-antd/src/bootstrap.ts Added watchEffect and preferences imports; implemented dynamic title updates in bootstrap function.
apps/web-antd/src/router/guard.ts Commented out dynamic title setting logic in setupCommonGuard.
apps/web-ele/src/bootstrap.ts Added watchEffect and preferences imports; implemented dynamic title updates in bootstrap function.
apps/web-ele/src/router/guard.ts Commented out dynamic title setting logic in setupCommonGuard.
apps/web-naive/src/bootstrap.ts Added watchEffect and preferences imports; implemented dynamic title updates in bootstrap function.
apps/web-naive/src/router/guard.ts Commented out dynamic title setting logic in setupCommonGuard.
playground/src/bootstrap.ts Added watchEffect and preferences imports; enhanced dynamic title updates in bootstrap function.
playground/src/router/guard.ts Commented out dynamic title setting logic in setupCommonGuard.

Possibly related PRs

  • perf: 优化 router/guard.ts  #4402: The changes in apps/web-antd/src/router/guard.ts involve the removal of dynamic title setting functionality, which is directly related to the title management logic introduced in the main PR's bootstrap.ts file.

Suggested labels

bug

Suggested reviewers

  • vince292007
  • anncwb

🐰 In the meadow, where titles bloom,
A rabbit hops to dispel the gloom.
With watchEffect, the titles will sway,
Reflecting the route in a vibrant display.
No more static, just dynamic delight,
Hopping along, our titles take flight! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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

🧹 Outside diff range and nitpick comments (3)
apps/web-naive/src/bootstrap.ts (1)

36-39: Consider adding error handling and type safety.

The title construction logic could be more robust with these improvements:

-      const routeTitle = router.currentRoute.value.meta?.title;
-      const pageTitle =
-        (routeTitle ? `${$t(routeTitle)} - ` : '') + preferences.app.name;
+      const route = router.currentRoute.value;
+      const routeTitle = route.meta?.title;
+      const pageTitle = [
+        routeTitle && typeof routeTitle === 'string' ? $t(routeTitle) : '',
+        preferences.app.name,
+      ].filter(Boolean).join(' - ');

This refactor:

  • Adds type checking for title
  • Uses array join for cleaner concatenation
  • Maintains the same functionality with better safety
apps/web-ele/src/bootstrap.ts (1)

34-42: Implementation looks good with room for improvement.

The watchEffect implementation correctly addresses the title update issue by reactively responding to language changes. Consider these defensive programming improvements:

 watchEffect(() => {
-  if (preferences.app.dynamicTitle) {
+  if (preferences?.app?.dynamicTitle) {
     const routeTitle = router.currentRoute.value.meta?.title;
+    if (routeTitle && typeof routeTitle !== 'string') {
+      console.warn('Route title should be a string:', routeTitle);
+      return;
+    }
     const pageTitle =
-      (routeTitle ? `${$t(routeTitle)} - ` : '') + preferences.app.name;
+      (routeTitle ? `${$t(routeTitle)} - ` : '') + (preferences?.app?.name ?? '');
     useTitle(pageTitle);
   }
 });
apps/web-antd/src/bootstrap.ts (1)

35-43: Consider adding cleanup for the watchEffect.

The implementation correctly addresses the title update issue using a reactive approach. However, consider stopping the watcher when the app is unmounted to prevent memory leaks.

 // 动态更新标题
-watchEffect(() => {
+const stopTitleWatch = watchEffect(() => {
   if (preferences.app.dynamicTitle) {
     const routeTitle = router.currentRoute.value.meta?.title;
     const pageTitle =
       (routeTitle ? `${$t(routeTitle)} - ` : '') + preferences.app.name;
     useTitle(pageTitle);
   }
 });
+
+// Add cleanup
+app.unmount(() => {
+  stopTitleWatch();
+});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 44440d0 and dec342e.

📒 Files selected for processing (8)
  • apps/web-antd/src/bootstrap.ts (2 hunks)
  • apps/web-antd/src/router/guard.ts (0 hunks)
  • apps/web-ele/src/bootstrap.ts (2 hunks)
  • apps/web-ele/src/router/guard.ts (0 hunks)
  • apps/web-naive/src/bootstrap.ts (2 hunks)
  • apps/web-naive/src/router/guard.ts (0 hunks)
  • playground/src/bootstrap.ts (2 hunks)
  • playground/src/router/guard.ts (0 hunks)
💤 Files with no reviewable changes (4)
  • apps/web-antd/src/router/guard.ts
  • apps/web-ele/src/router/guard.ts
  • apps/web-naive/src/router/guard.ts
  • playground/src/router/guard.ts
🔇 Additional comments (8)
apps/web-naive/src/bootstrap.ts (2)

1-1: LGTM: Required imports for dynamic title functionality.

The added imports provide the necessary functionality for reactive title updates.

Also applies to: 4-4, 8-8


33-41: LGTM: Effective solution for dynamic title updates.

The watchEffect implementation successfully addresses the issue of title not updating after language switch. The reactive nature ensures the title updates whenever the language or route changes.

apps/web-ele/src/bootstrap.ts (2)

1-1: LGTM! Import changes are appropriate.

The new imports are correctly added to support the dynamic title update functionality.

Also applies to: 4-4, 9-9, 11-11


34-42: Verify title updates across all supported languages.

Let's ensure the title updates work correctly for all configured languages.

✅ Verification successful

Let me gather more information about the i18n implementation and available translations.


Based on the gathered information, I can now provide the final response:

Title updates are properly implemented for all supported languages (zh-CN and en-US)

The dynamic title update implementation is working correctly because:

  • The i18n system is properly configured with support for multiple languages (zh-CN and en-US)
  • Translation files are organized by language in packages/locales/src/langs/
  • The $t() function is correctly used to translate route titles
  • The same title update logic is consistently implemented across all app variants (web-antd, web-ele, web-naive)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all language configurations and translation keys for route titles

# Find language configuration files
echo "Searching for language configuration files..."
fd -e json -e yaml -e ts "locale|lang|i18n"

# Find route configurations with title meta
echo "Searching for route title configurations..."
rg -A 2 "meta.*title" 

Length of output: 4739


Script:

#!/bin/bash
# Check i18n implementation and translations

# Check the i18n configuration
echo "Checking i18n implementation..."
cat packages/locales/src/i18n.ts

# Look for translation files
echo -e "\nSearching for translation files..."
fd -e json -e yaml -e ts "zh|en|locale|lang" packages/locales/

# Check for title-related translations
echo -e "\nSearching for title-related translations..."
rg -A 1 "title.*:" packages/locales/

Length of output: 12064

apps/web-antd/src/bootstrap.ts (2)

1-1: LGTM: Import statements are well-organized and necessary.

The new imports support the dynamic title management functionality and follow a logical organization pattern.

Also applies to: 4-4, 9-11


38-41: Verify translation key handling.

Please ensure that all route meta titles have corresponding translations to avoid displaying raw translation keys.

playground/src/bootstrap.ts (2)

1-1: LGTM: Import statements are properly organized

The new imports are correctly added and organized, providing all necessary dependencies for the dynamic title update functionality.

Also applies to: 4-4, 10-10, 12-13


39-47: LGTM: Dynamic title update implementation effectively fixes the language switch issue

The implementation correctly uses watchEffect to react to language changes and updates the title accordingly. The code is clean and follows Vue best practices.

Consider adding some defensive programming:

 watchEffect(() => {
   if (preferences.app.dynamicTitle) {
     const routeTitle = router.currentRoute.value.meta?.title;
+    // Skip update if no app name is configured
+    if (!preferences.app.name) return;
+    
     const pageTitle =
-      (routeTitle ? `${$t(routeTitle)} - ` : '') + preferences.app.name;
+      (routeTitle ? `${$t(String(routeTitle))} - ` : '') + preferences.app.name;
     useTitle(pageTitle);
   }
 });

Let's verify this fixes the original issue by checking for any other title-related code:

✅ Verification successful

Title update implementation is consistent across all web apps

The verification shows that the dynamic title update logic is consistently implemented across all web applications (web-naive, web-antd, web-ele, playground) using the same pattern:

  • All use useTitle from @vueuse/core
  • All follow the same reactive pattern with route meta titles
  • No direct document.title manipulations found
  • Route meta titles are properly used in the routing configuration

The implementation is verified to be correct and consistent, ensuring proper title updates when language changes across the entire codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any other title-setting logic that might interfere
# Expected: No other direct document.title manipulations or useTitle calls

# Search for title-related code
echo "Searching for other title manipulations..."
rg -g '!*.md' -g '!*.json' "document\.title|useTitle" 

# Search for related route meta title usage
echo "Checking route title usage..."
rg "meta.*title" -g '*.ts' -g '*.vue'

Length of output: 2563

@vince292007 vince292007 merged commit 488ccb5 into vbenjs:main Nov 5, 2024
14 checks passed
@mynetfan mynetfan deleted the fix/page-title branch November 5, 2024 12:35
@anncwb anncwb added the bug Something isn't working label Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants