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

Jump to bottom

fix: 修复 tour 组件无法根据 current 值展示指定步骤以及 change 事件得到的值有误 #3139

Open
wants to merge 3 commits into
base: v4
Choose a base branch
from
Open

fix: 修复 tour 组件无法根据 current 值展示指定步骤以及 change 事件得到的值有误 #3139

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd17a40
fix: 修复 tour 组件无法根据 current 指定展示的步骤以及 change 事件得到的值有误
daiwanxing Jul 12, 2024
43cf731
perf: 优化 current 的边界值判断
daiwanxing Jul 12, 2024
5f7c046
chore: 移除 CSSProperties 类型
daiwanxing Jul 31, 2024
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
70 changes: 27 additions & 43 deletions src/packages/__VUE/tour/index.taro.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<template>
<view :class="classes">
<view class="nut-tour">
<view v-if="showTour" class="nut-tour-masked" @click="handleClickMask"></view>

<view v-for="(step, i) in steps" :key="i" style="height: 0">
Expand Down Expand Up @@ -42,17 +42,15 @@
@click="changeStep('prev')"
>
{{ prevStepTxt }}
</view
>
</view>
Copy link

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>
Copy link

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>


<slot name="next-step">
<view
Expand All @@ -61,8 +59,7 @@
@click="changeStep('next')"
>
{{ nextStepTxt }}
</view
>
</view>
Copy link

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>

</slot>
</view>
</view>
Expand All @@ -80,7 +77,7 @@
</view>
</template>
<script lang="ts">
import { computed, watch, ref, reactive, toRefs, PropType, onMounted } from 'vue'
import { watch, ref, reactive, toRefs, PropType, onMounted, CSSProperties } from 'vue'
import { PopoverLocation, PopoverTheme } from '../popover/type'
import { createComponent } from '@/packages/utils/create'
import { rectTaro, useTaroRectById } from '@/packages/utils/useTaroRect'
Expand All @@ -96,7 +93,7 @@
}
export type TourType = 'step' | 'tile'
const { create } = createComponent('tour')
export default create({

Check failure on line 96 in src/packages/__VUE/tour/index.taro.vue

View workflow job for this annotation

GitHub Actions / build 

The inferred type of 'default' cannot be named without a reference to '.pnpm/[email protected]/node_modules/csstype'. This is likely not portable. A type annotation is necessary.
components: {
NutPopover,
Close
Expand Down Expand Up @@ -179,17 +176,12 @@

let maskRect: rectTaro[] = []

let maskStyles = ref<any[]>([])

const classes = computed(() => {
const prefixCls = 'nut-tour'
return `${prefixCls}`
})
let maskStyles = ref<CSSProperties[]>([])

const maskStyle = (index: number) => {
const { offset, maskWidth, maskHeight } = props

if (!maskRect[index]) return {}
if (!maskRect[index]) return
const { width, height, left, top } = maskRect[index]

const center = [left + width / 2, top + height / 2] // 中心点 【横，纵】
Expand All @@ -209,31 +201,26 @@
const current = state.active
let next = current

if (type == 'next') {
next = current + 1
} else {
next = current - 1
}
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
Comment on lines +204 to +210
Copy link

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.

Suggested change
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)

}, 300)

emit('change', state.active)
}

const getRootPosition = () => {
props.steps.forEach(async (item, i) => {
useTaroRectById(item.target).then(
(rect: any) => {
maskRect[i] = rect
maskStyle(i)
},
() => {}
)
})
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)
}
}
Comment on lines +214 to +223
Copy link

@coderabbitai coderabbitai bot Jul 12, 2024 •
edited
Loading

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.

Suggested change
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)
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里拿不到 step 的位置信息，再赋值默认值有必要吗

Copy link

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 位置信息无法获取的情况？如果没有问题，我们可以考虑不赋值默认值。

}

const close = () => {
Expand All @@ -248,33 +235,30 @@
}

onMounted(() => {
setTimeout(() => {
getRootPosition()
}, 500)
getRootPosition()
})

watch(
() => props.modelValue,
(val) => {
if (val) {
state.active = 0
(visible) => {
if (visible) {
state.active = props.type === 'step' ? Math.min(Math.max(0, props.current), props.steps.length - 1) : 0
getRootPosition()
}
state.showTour = val
showPopup.value[state.active] = val
state.showTour = visible
showPopup.value[state.active] = visible
}
)

const refRandomId = Math.random().toString(36).slice(-8)

return {
...toRefs(state),
classes,
maskStyle,
changeStep,
showPopup,
close,
handleClickMask,
showPopup,
maskStyles,
refRandomId
}
Expand Down
Loading