Skip to content

Comments

feat(minapps): add proper region/language tags for all mini apps#12636

Open
GeorgeDong32 wants to merge 16 commits intomainfrom
fix/minapps-region-tags
Open

feat(minapps): add proper region/language tags for all mini apps#12636
GeorgeDong32 wants to merge 16 commits intomainfrom
fix/minapps-region-tags

Conversation

@GeorgeDong32
Copy link
Collaborator

@GeorgeDong32 GeorgeDong32 commented Jan 28, 2026

What this PR does

为所有小程序添加了正确的地区(supportedRegions)和语言(locales)标签,并优化了过滤逻辑。

Before this PR

  • 部分小程序缺少 supportedRegions 和 locales 标签
  • 用户手动固定的地区限制小程序在切换地区后会消失

After this PR

  • 所有小程序都配置了正确的 supportedRegions 和 locales 标签
  • 用户主动固定的小程序不再受地区/语言过滤影响,始终显示

Why we need it

  1. 地区过滤: Global 用户不应该看到仅中国可用的小程序
  2. 语言过滤: 用户应该看到支持其界面语言的小程序
  3. 用户体验: 用户固定的小程序代表明确意图,不应被过滤

Special notes for your reviewer

主要修改文件:

  • src/renderer/src/config/minapps.ts: 添加/修正小程序标签
  • src/renderer/src/hooks/useMinapps.ts: 优化过滤逻辑,pinned apps 不受过滤影响

Release note

feat(minapps): 已添加所有小程序地区/语言标签,固定小程序不再受地区过滤影响

@GeorgeDong32 GeorgeDong32 marked this pull request as draft January 28, 2026 14:24
@DeJeune DeJeune added this to the v1.7.16 milestone Jan 29, 2026
@GeorgeDong32 GeorgeDong32 force-pushed the fix/minapps-region-tags branch from 7e41373 to 6ff80a3 Compare January 30, 2026 13:30
@GeorgeDong32 GeorgeDong32 marked this pull request as ready for review January 30, 2026 15:20
@GeorgeDong32 GeorgeDong32 requested a review from EurFelux January 31, 2026 12:34
@GeorgeDong32 GeorgeDong32 force-pushed the fix/minapps-region-tags branch from 1097d14 to 30b76e3 Compare February 2, 2026 05:01
@GeorgeDong32 GeorgeDong32 requested a review from DeJeune February 2, 2026 05:09
@EurFelux
Copy link
Collaborator

EurFelux commented Feb 2, 2026

Note

This issue/comment/review was translated by Claude.

📝 Code Review Report

Thanks to @GeorgeDong32 for the contribution! This is a valuable feature. After review, I found the following issues:


🔴 Issues that need fixing

1. Custom mini apps lack supportedRegions handling

File: src/renderer/src/config/minapps.ts:78-83

When custom mini apps are loaded, the supportedRegions field is not preserved, causing them to be treated as CN-only (according to isVisibleForRegion logic), and Global users will not be able to see custom mini apps.

Suggested fix:

return customApps.map((app: any) => ({
  ...app,
  type: 'Custom',
  logo: app.logo && app.logo !== '' ? app.logo : ApplicationLogo,
  addTime: app.addTime || now,
  supportedRegions: app.supportedRegions || ['CN', 'Global'] // visible to everyone by default
}))

✅ Correctly implemented parts

Item Status Description
Pinned mini app filtering logic pinnedApps correctly excludes region filtering, matching PR description
Type definitions MinAppType correctly defines the supportedRegions field
Region filtering logic CN users see all, Global users only see apps marked 'Global'
IP detection service Has timeout control, defaults to 'CN' on error (conservative strategy)
IPC channel App_GetIpCountry correctly added
Data flow design Redux stores all, Hook filters on read, merges on write

🟡 Optional improvements

  1. Add logging: According to project guidelines, it's recommended to use loggerService to log key operations (such as region detection completion)

📋 Testing suggestions

  1. Test whether custom mini apps are visible in Global environment
  2. Test whether pinned mini apps still display after switching regions
  3. Test the fallback behavior when IP detection fails

Overall assessment: The code architecture design is reasonable, the main issue is handling edge cases for custom mini apps. Can be merged after fixing.

/label ~"needs-fix"


Original Content

📝 Code Review Report

感谢 @GeorgeDong32 的贡献!这是一个很有价值的功能。经过审查,我发现以下问题:


🔴 需要修复的问题

1. 自定义小程序缺少 处理

文件:

自定义小程序加载时未保留 字段,导致它们会被视为 CN-only(根据 逻辑),Global 用户将无法看到自定义小程序。

建议修复:


✅ 正确实现的部分

项目 状态 说明
固定小程序过滤逻辑 正确排除了地区过滤,符合 PR 描述
类型定义 正确定义了 字段
地区过滤逻辑 CN 用户看到全部,Global 用户只 see 'Global' 标记的应用
IP 检测服务 有超时控制,错误时默认返回 'CN'(保守策略)
IPC 通道 正确添加
数据流设计 Redux 存储全部,Hook 在读取时过滤,写入时合并

🟡 可选改进

  1. 添加日志记录: 根据项目规范,建议使用 记录关键操作(如地区检测完成)

📋 测试建议

  1. 测试自定义小程序在 Global 环境下是否可见
  2. 测试固定小程序在切换地区后是否仍然显示
  3. 测试 IP 检测失败时的回退行为

总体评价: 代码架构设计合理,主要问题是自定义小程序的边界情况处理。修复后即可合并。

/label ~"needs-fix"

@GeorgeDong32
Copy link
Collaborator Author

Done

@EurFelux
Copy link
Collaborator

EurFelux commented Feb 3, 2026

🔍 Code Review: Mini App Region/Language Tags

📋 Summary

This PR adds proper region-based filtering for mini apps, replacing the previous locale-based approach. It introduces IP-based region detection and ensures pinned apps remain visible regardless of region settings.

✅ What's Good

1. Clean Architecture

  • Well-separated concerns: detection logic in useMinapps.ts, IP service in main process
  • Module-level promise caching prevents duplicate IP detection requests
  • Proper Redux state management with both persistent settings and runtime state

2. User Experience Improvements

  • Pinned apps are now always visible (respects user intent) - MinApp.tsx:57-58
  • Custom mini apps default to ['CN', 'Global'] visibility - minapps.ts:81
  • Conservative fallback to 'CN' when detection fails - useMinapps.ts:708-709

3. Internationalization

  • Comprehensive i18n support across 11 languages
  • Consistent translation keys following existing patterns

4. Type Safety

  • New types MinAppRegion and MinAppRegionFilter properly defined
  • Type-safe IPC channel additions

⚠️ Suggestions & Questions

1. API Token Exposure
The IP detection service uses a hardcoded token:

// src/main/utils/ipService.ts:16
const ipinfo = await net.fetch(`https://api.ipinfo.io/lite/me?token=2a42580355dae4`, ...)

Question: Is this a public/free token or should it be configurable? Consider documenting the rate limits or making it an environment variable for self-hosted builds.

2. Region Detection Reliability

// useMinapps.ts:708-709
} catch {
  return 'CN'  // Conservative approach
}

The fallback to 'CN' shows all apps, which is good for compatibility. However, users behind strict firewalls (corporate networks, certain countries) may consistently fail detection. Consider:

  • Adding a manual region selector in settings (✅ already implemented)
  • Caching the last successful detection

3. Migration Path for Existing Users
The PR removes locales field from MinAppType. Existing user data with locales will be ignored. Consider if any migration is needed for users who had custom mini apps with locale settings.

4. Comment Consistency
Some comments are in Chinese while the codebase appears to use English. Consider standardizing:

// MiniAppSettings.tsx:1050
// 导入重置图标和Info图标

🎯 Code Quality: 8/10

Aspect Score Notes
Architecture 9/10 Clean separation of concerns
Type Safety 9/10 Good TypeScript coverage
Testing N/A No tests added (consider adding)
Documentation 7/10 Good inline comments, some in Chinese
i18n 10/10 Comprehensive translations

🚀 Recommendation: Approve with minor suggestions

The implementation is solid and addresses the user experience issues described in the PR. The region-based approach is more practical than the previous locale-based filtering.

Before merge, consider:

  1. Documenting the ipinfo.io token usage/limitations
  2. Verifying the localessupportedRegions migration doesn't break existing user configs

Reviewed with ❤️ by Claude Code

@kangfenmao
Copy link
Collaborator

image

@GeorgeDong32 GeorgeDong32 force-pushed the fix/minapps-region-tags branch from 803beed to f499c46 Compare February 3, 2026 15:25
@GeorgeDong32
Copy link
Collaborator Author

Done

@GeorgeDong32 GeorgeDong32 force-pushed the fix/minapps-region-tags branch 2 times, most recently from 972a777 to 1330bb2 Compare February 6, 2026 16:15
GeorgeDong32 and others added 13 commits February 7, 2026 21:13
Ensure pinned mini-apps remain rendered even when region or
locale filtering would otherwise hide them. Compute shouldShow
as a union of visibility and pinned state, and use it to guard
rendering, so pinned items stay accessible and pinned UI state
is preserved (opened/active indicators) regardless of filters.
Co-authored-by: Phantom <eurfelux@gmail.com>
Co-authored-by: Phantom <eurfelux@gmail.com>
…sers

- Add migration 194 to initialize minAppRegion setting to auto
- Add defensive default value in RegionSelector component

Fixes: Region selector showing placeholder instead of default value
@GeorgeDong32 GeorgeDong32 force-pushed the fix/minapps-region-tags branch from 1330bb2 to b41c3f9 Compare February 7, 2026 13:13
@EurFelux
Copy link
Collaborator

EurFelux commented Feb 9, 2026

@0xfullex review needed

@GeorgeDong32 GeorgeDong32 linked an issue Feb 17, 2026 that may be closed by this pull request
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants