-
Notifications
You must be signed in to change notification settings - Fork 267
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(uploader): use chooseImage API when env ='JD' #2652
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2652 +/- ##
=======================================
Coverage 84.07% 84.07%
=======================================
Files 217 217
Lines 17827 17827
Branches 2608 2608
=======================================
Hits 14988 14988
Misses 2834 2834
Partials 5 5 ☔ View full report in Codecov by Sentry. |
Walkthrough此次更改主要集中在 Changes
Possibly related PRs
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
@@ -260,7 +260,7 @@ const InternalUploader: ForwardRefRenderFunction< | |||
document.body.appendChild(obj) | |||
} | |||
} | |||
if ((getEnv() === 'WEAPP' || getEnv() === 'JD') && chooseMedia) { | |||
if (getEnv() === 'WEAPP' && chooseMedia) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
发现其他文件中仍存在 'JD' 环境检查,请检查并确保一致性。
- src/packages/signature/signature.taro.tsx
🔗 Analysis chain
修改看起来符合预期,但建议进行额外验证。
这个改动移除了对 'JD' 环境的特殊处理,现在只有在 'WEAPP' 环境下才会使用 chooseMedia
API。这符合 PR 的目标,即在 'JD' 环境下使用 chooseImage
API。
建议在 'JD' 环境中测试此更改,以确保 chooseImage
API 能够正常工作,并且没有引入任何意外的副作用。可以运行以下脚本来检查是否有其他地方可能受到影响:
如果脚本返回结果,请检查这些位置是否也需要类似的更改。
建议调整搜索命令以正确识别 .ts
和 .tsx
文件。可以使用 --glob
选项来指定文件扩展名。以下是更新后的脚本:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查代码库中是否有其他地方使用了 'JD' 环境检查
rg "getEnv\(\)\s*===\s*('|\")JD('|\")" --type tsx --type ts
Length of output: 93
Script:
#!/bin/bash
# 描述:检查代码库中是否有其他地方使用了 'JD' 环境检查
rg "getEnv\(\)\s*===\s*('|\")JD('|\")" --glob '*.ts' --glob '*.tsx'
Length of output: 269
Summary by CodeRabbit
chooseMedia
,其他环境继续使用chooseImage
。maxCount
的计算,确保用户选择的文件数量符合最大限制。