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.

Sign up for GitHub

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(rn): 修复previewImage 预览图片API的current必须是urls其中的一个 #11632

Open
wants to merge 8 commits into
base: 3.x
Choose a base branch
from
Open

fix(rn): 修复previewImage 预览图片API的current必须是urls其中的一个 #11632

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
59639fe
fix: 修复previewImage 预览图片API的current必须是urls其中的一个
Apr 13, 2022
34088cc
Merge branch 'next' into feature-fix-previewImage
zhiqingchen Apr 13, 2022
c00c1e7
fix: 优化previewImage预览图片api的传参校验逻辑
Apr 19, 2022
d34770c
Merge branch 'next' into feature-fix-previewImage
zhiqingchen Apr 19, 2022
50d7845
fix: 优化previewImage预览图片api的传参校验逻辑
Apr 20, 2022
c05caab
Merge branch 'feature-fix-previewImage' of https://github.com/WhiteSe…
Apr 20, 2022
7e872cb
fix: 优化预览图片校验逻辑
Apr 21, 2022
c668232
Merge branch 'next' into feature-fix-previewImage
zhiqingchen Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/taro-rn/src/lib/previewImage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ export function previewImage(obj: Taro.previewImage.Option): void {
fail,
complete
}: any = obj || {}
const index = urls.indexOf(current)
if (index === -1) {

let index = 0
if (urls.length === 0) {
throw new Error('"current" or "urls" is invalid')
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里把提示文案给改下吧

} else {
index = Math.max(current, urls.indexOf(current))
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里的 urls.indexOf(current) 可以先声明出来，然后 === -1 时给个 warning 信息（console.warn('xxxx xxxx xxxx')），不等于 -1 时再取 max。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

突然发现不能取max,因为curreny是一个字符串，取max是返回的NaN
改成这样看是否可以？

let index = 0
if (!urls || urls.length === 0) {
  throw new Error('待预览的图片列表"urls"不能为空')
} else {
  urls.indexOf(current) === -1 ? console.warn('显示的图片不在预览列表当中') : ''
  index = Math.max(0, urls.indexOf(current))
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 提示文案翻译成英文；
  2. 下面一段你看看这样写呢？少执行一次 indexOf
const _index = urls.indexOf(current)
if (_index === -1) {
  console.warn('xxxx')
}
index = Math.max(0, _index)

}

Copy link
Collaborator

Choose a reason for hiding this comment

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

修改后把 commit 合并成一个就行了

Copy link
Member

Choose a reason for hiding this comment

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

这个我们 squash and merge 就会是一个commit

Copy link
Collaborator

Choose a reason for hiding this comment

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

哦哦 我以为要先合并呢😂

let sibling
Expand Down