-
Notifications
You must be signed in to change notification settings - Fork 835
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: 修复 tour 组件无法根据 current 值展示指定步骤以及 change 事件得到的值有误 #3139
base: v4
Are you sure you want to change the base?
Conversation
Walkthrough这些更改集中在 Changes
Poem
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 as PR comments)
Additionally, you can add 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: 3
Outside diff range, codebase verification and nitpick comments (2)
src/packages/__VUE/tour/index.taro.vue (2)
2-2
: 建议:优化类绑定方式。将动态类绑定改为静态类分配的方式可能会导致某些条件下的样式无法正确应用。建议确认这样更改是否符合预期。
80-80
: 优化导入顺序。建议将
CSSProperties
放在其他导入类型之前,以保持导入顺序的一致性和可读性。import { CSSProperties, watch, ref, reactive, toRefs, PropType, onMounted } from 'vue'
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/packages/__VUE/tour/index.taro.vue (7 hunks)
Additional comments not posted (5)
src/packages/__VUE/tour/index.taro.vue (5)
179-179
: 更改类型定义。将
maskStyles
的类型从ref<any[]>
更改为ref<CSSProperties[]>
提高了类型安全性,这是一个很好的改进。
184-184
: 检查索引是否存在。在访问
maskRect[index]
之前,建议确保索引存在,以避免潜在的运行时错误。if (index in maskRect && !maskRect[index]) return
204-210
: 改进步骤切换逻辑。改进后的
changeStep
函数逻辑更清晰,确保了步骤切换的正确性。
214-223
: 改进根元素位置获取逻辑。改进后的
getRootPosition
函数逻辑更清晰,捕获错误的方式更好。
238-249
: 改进属性观察逻辑。改进后的
watch
函数逻辑更清晰,确保了组件状态的正确性。
@@ -42,17 +42,15 @@ | |||
@click="changeStep('prev')" | |||
> | |||
{{ prevStepTxt }} | |||
</view | |||
> | |||
</view> |
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.
建议:简化条件渲染。
可以将 active != 0
和 showPrevStep
的条件合并,减少不必要的 DOM 操作。
<view v-if="active != 0 && showPrevStep" class="nut-tour-content-bottom-operate-btn" @click="changeStep('prev')">
{{ prevStepTxt }}
</view>
</slot> | ||
<view | ||
v-if="steps.length - 1 == active" | ||
class="nut-tour-content-bottom-operate-btn active" | ||
@click="close" | ||
> | ||
{{ completeTxt }} | ||
</view | ||
> | ||
</view> |
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.
建议:简化条件渲染。
可以将 steps.length - 1 == active
的条件直接放在 v-if
中,减少不必要的 DOM 操作。
<view v-if="steps.length - 1 == active" class="nut-tour-content-bottom-operate-btn active" @click="close">
{{ completeTxt }}
</view>
@@ -61,8 +59,7 @@ | |||
@click="changeStep('next')" | |||
> | |||
{{ nextStepTxt }} | |||
</view | |||
> | |||
</view> |
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.
建议:简化条件渲染。
可以将 steps.length - 1 != active
的条件直接放在 v-if
中,减少不必要的 DOM 操作。
<view v-if="steps.length - 1 != active" class="nut-tour-content-bottom-operate-btn active" @click="changeStep('next')">
{{ nextStepTxt }}
</view>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v4 #3139 +/- ##
==========================================
- Coverage 81.20% 81.18% -0.02%
==========================================
Files 226 226
Lines 18139 18142 +3
Branches 2485 2485
==========================================
Hits 14729 14729
- Misses 3392 3395 +3
Partials 18 18 ☔ View full report in Codecov by Sentry. |
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: 2
Outside diff range, codebase verification and nitpick comments (1)
src/packages/__VUE/tour/index.taro.vue (1)
80-80
: 优化建议:移除未使用的 import。可以移除
CSSProperties
的导入,因为它未被使用。- import { watch, ref, reactive, toRefs, PropType, onMounted, CSSProperties } from 'vue' + import { watch, ref, reactive, toRefs, PropType, onMounted } from 'vue'
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/packages/__VUE/tour/index.taro.vue (7 hunks)
Additional comments not posted (4)
src/packages/__VUE/tour/index.taro.vue (4)
Line range hint
2-45
: 建议:简化条件渲染。可以将
active != 0
和showPrevStep
的条件合并,减少不必要的 DOM 操作。<view v-if="active != 0 && showPrevStep" class="nut-tour-content-bottom-operate-btn" @click="changeStep('prev')"> {{ prevStepTxt }} </view>
53-53
: 建议:简化条件渲染。可以将
steps.length - 1 == active
的条件直接放在v-if
中,减少不必要的 DOM 操作。<view v-if="steps.length - 1 == active" class="nut-tour-content-bottom-operate-btn active" @click="close"> {{ completeTxt }} </view>
62-62
: 建议:简化条件渲染。可以将
steps.length - 1 != active
的条件直接放在v-if
中,减少不必要的 DOM 操作。<view v-if="steps.length - 1 != active" class="nut-tour-content-bottom-operate-btn active" @click="changeStep('next')"> {{ nextStepTxt }} </view>
179-179
: 优化建议:更新类型定义。更新
maskStyles
的类型定义以提高代码的可读性和类型检查的准确性。let maskStyles = ref<CSSProperties[]>([])
next = type == 'next' ? current + 1 : current - 1 | ||
showPopup.value[current] = false | ||
|
||
setTimeout(() => { | ||
showPopup.value[next] = true | ||
state.active = next | ||
emit('change', state.active) | ||
showPopup.value[state.active] = true |
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.
优化建议:改进 changeStep
方法。
在 changeStep
方法中,可以在 next
变量赋值之前添加边界条件检查,以防止数组越界。
+ if (type === 'next' && current >= props.steps.length - 1) return
+ if (type === 'prev' && current <= 0) return
next = type == 'next' ? current + 1 : current - 1
showPopup.value[current] = false
setTimeout(() => {
state.active = next
emit('change', state.active)
showPopup.value[state.active] = true
}, 300)
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
next = type == 'next' ? current + 1 : current - 1 | |
showPopup.value[current] = false | |
setTimeout(() => { | |
showPopup.value[next] = true | |
state.active = next | |
emit('change', state.active) | |
showPopup.value[state.active] = true | |
if (type === 'next' && current >= props.steps.length - 1) return | |
if (type === 'prev' && current <= 0) return | |
next = type == 'next' ? current + 1 : current - 1 | |
showPopup.value[current] = false | |
setTimeout(() => { | |
state.active = next | |
emit('change', state.active) | |
showPopup.value[state.active] = true | |
}, 300) |
const getRootPosition = async () => { | ||
for (const [index, step] of props.steps.entries()) { | ||
try { | ||
const rect = await useTaroRectById(step.target) | ||
maskRect[index] = rect as rectTaro | ||
maskStyle(index) | ||
} catch (error) { | ||
console.warn(`[NutUI] Failed to get rect for step ${index}:`, error) | ||
} | ||
} |
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.
建议:增加错误处理。
在 getRootPosition
方法中,可以增加更多的错误处理逻辑,以确保每个步骤的 rect 都能正确获取或处理错误情况。
const getRootPosition = async () => {
for (const [index, step] of props.steps.entries()) {
try {
const rect = await useTaroRectById(step.target)
maskRect[index] = rect as rectTaro
maskStyle(index)
} catch (error) {
console.warn(`[NutUI] Failed to get rect for step ${index}:`, error)
+ maskRect[index] = { width: 0, height: 0, left: 0, top: 0 } // 默认值
+ maskStyle(index)
}
}
}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const getRootPosition = async () => { | |
for (const [index, step] of props.steps.entries()) { | |
try { | |
const rect = await useTaroRectById(step.target) | |
maskRect[index] = rect as rectTaro | |
maskStyle(index) | |
} catch (error) { | |
console.warn(`[NutUI] Failed to get rect for step ${index}:`, error) | |
} | |
} | |
const getRootPosition = async () => { | |
for (const [index, step] of props.steps.entries()) { | |
try { | |
const rect = await useTaroRectById(step.target) | |
maskRect[index] = rect as rectTaro | |
maskStyle(index) | |
} catch (error) { | |
console.warn(`[NutUI] Failed to get rect for step ${index}:`, error) | |
maskRect[index] = { width: 0, height: 0, left: 0, top: 0 } // 默认值 | |
maskStyle(index) | |
} | |
} |
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.
这里拿不到 step
的位置信息,再赋值默认值有必要吗
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.
@daiwanxing 确定给 step
赋值默认值的目的是为了防止在无法获取位置信息时,应用程序出现意外行为或错误。这些默认值可以作为一种安全措施,确保 maskStyle
方法能够正常执行。
不过,如果在实际使用中没有遇到因为 step
位置信息无法获取而导致的问题,那么这些默认值可能确实没有必要。请问您在使用中是否遇到过 step
位置信息无法获取的情况?如果没有问题,我们可以考虑不赋值默认值。
这个错误麻烦帮我看看原因 @eiinu |
似乎在当前组件内定义与 CSSProperties 类型有关的变量时都会报错,没有排查出具体的原因,可能与定义组件的方法有关系,用 any 暂时绕过吧。后续切换成 script + setup 应该就不会有这个问题了。 |
好的,调整了 |
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/packages/__VUE/tour/index.taro.vue (7 hunks)
Additional comments not posted (3)
src/packages/__VUE/tour/index.taro.vue (3)
204-210
: 建议:改进changeStep
方法。在
changeStep
方法中,可以在next
变量赋值之前添加边界条件检查,以防止数组越界。+ if (type === 'next' && current >= props.steps.length - 1) return + if (type === 'prev' && current <= 0) return next = type == 'next' ? current + 1 : current - 1 showPopup.value[current] = false setTimeout(() => { state.active = next emit('change', state.active) showPopup.value[state.active] = true }, 300)
214-223
: 建议:增加错误处理。在
getRootPosition
方法中,可以增加更多的错误处理逻辑,以确保每个步骤的 rect 都能正确获取或处理错误情况。const getRootPosition = async () => { for (const [index, step] of props.steps.entries()) { try { const rect = await useTaroRectById(step.target) maskRect[index] = rect as rectTaro maskStyle(index) } catch (error) { console.warn(`[NutUI] Failed to get rect for step ${index}:`, error) + maskRect[index] = { width: 0, height: 0, left: 0, top: 0 } // 默认值 + maskStyle(index) } } }
243-249
: LGTM! 但是请验证函数的使用。代码更改已批准。
不过,请确保所有对
watch
函数的调用都匹配新的逻辑。Verification successful
LGTM! 但是请验证函数的使用。
代码更改已批准。没有发现其他对
props.modelValue
的watch
调用。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `watch` match the new logic. # Test: Search for the function usage. Expect: Only occurrences of the new logic. rg --type vue -A 5 $'watch'Length of output: 54
Script:
#!/bin/bash # Description: Verify all function calls to `watch` match the new logic. # Find all .vue files and search for the watch function within them fd -e vue -x rg 'watch' {}Length of output: 9121
Script:
#!/bin/bash # Description: Verify all function calls to `watch` match the new logic. # Find all .vue files and search for watch functions that observe props.modelValue fd -e vue -x rg 'watch\(\(\) => props\.modelValue' {}Length of output: 57
Script:
#!/bin/bash # Description: Verify all function calls to `watch` match the new logic. # Find all .vue files and search for watch functions that observe props fd -e vue -x rg 'watch\((\(\) => props' {}Length of output: 98286
Script:
#!/bin/bash # Description: Verify all function calls to `watch` match the new logic. # Find all .vue files and search for watch functions that observe props fd -e vue -x rg 'watch\(\(\) => props' {}Length of output: 136
这个 PR 做了什么? (简要描述所做更改)
修复了当用户传入
current
prop 时无效,组件未对current
做处理,还修复了onChange
事件得到的step
的值计算错误这个 PR 是什么类型? (至少选择一个)
这个 PR 涉及以下平台:
这个 PR 是否已自测:
Summary by CodeRabbit
Summary by CodeRabbit