feat(tinyvue): 代码风格格式化、逻辑优化 #186
Conversation
WalkthroughThis PR introduces TypeScript interfaces for API types, refactors component prop definitions to use destructured syntax and typed generics, decomposes complex nested components into lazy-loaded async components, restructures UI layouts in filter components, and applies extensive formatting normalization including style block cleanup across 40+ Vue files throughout the template directory. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
template/tinyvue/src/views/not-found/index.vue (1)
7-16: Theback()function is now a no-op, leaving dead code.The navigation to Home is commented out, but
onMountedstill callsback()after a 1-second timeout. This results in:
- A function that does nothing being called
- Confusing behavior where users see "404" but nothing happens
Either restore the navigation or remove the dead code entirely:
Option 1: Remove dead code if auto-redirect is not desired
<script lang="ts" setup> -import { onMounted } from 'vue' -import { useRouter } from 'vue-router' - -const router = useRouter() - -function back() { - // warning： Go to the node that has the permission - // router.push({ name: 'Home' }) -} - -onMounted(() => { - setTimeout(() => { - back() - }, 1000) -}) </script>Option 2: Restore navigation if auto-redirect is intended
function back() { // warning： Go to the node that has the permission - // router.push({ name: 'Home' }) + router.push({ name: 'Home' }) }template/tinyvue/src/api/list.ts (1)
52-56: Use theUpdateEmployeeInfotype instead ofany.The
UpdateEmployeeInfointerface was added in this PR but isn't being used here. This defeats the purpose of adding the type.-export function updateEmployeeInfo(data: any) { +export function updateEmployeeInfo(data: UpdateEmployeeInfo) { return axios.post( `${import.meta.env.VITE_MOCK_SERVER_HOST}/api/employee/updateEmployeeInfo`, { data }, ) }template/tinyvue/src/views/user/info/components/info-tab.vue (1)
103-105: Add optional chaining for async component reset call.
filterInforeferences an async component (InfoFilter). If this watcher triggers before the component has loaded,filterInfo.value.reset()will throw. Other reset calls ininfo-filter.vueuse optional chaining for this reason.🐛 Proposed fix
watch(activeName, () => { - filterInfo.value.reset() + filterInfo.value?.reset() })template/tinyvue/src/views/user/setting/components/set-from.vue (1)
90-100: Potential runtime error when accessinggetTime()on undefined values.If
state.filterOptions.startTimeorstate.filterOptions.endTimeis undefined or not a Date object, calling.getTime()will throw an error. Add null checks before the comparison.🐛 Proposed fix
function handleBlur() { - const start = state.filterOptions.startTime?.getTime() - const end = state.filterOptions.endTime?.getTime() - if (end < start) { + const startTime = state.filterOptions.startTime + const endTime = state.filterOptions.endTime + if (!startTime || !endTime) return + const start = startTime.getTime() + const end = endTime.getTime() + if (end < start) { state.filterOptions.endTime = '' Modal.message({ message: t('userInfo.time.message'), status: 'error', }) } }template/tinyvue/src/views/user/info/components/info-chart.vue (1)
64-64: Suspicious negative border width value.
border-right: -0.1pxuses a negative value which is invalid CSS. Negative border widths are not supported and will be ignored by browsers. If the intent is to hide the border, use0ornoneinstead.🐛 Proposed fix
- border-right: -0.1px solid `#d9d9d9`; + border-right: none;Or if a thin border is intended:
- border-right: -0.1px solid `#d9d9d9`; + border-right: 0.5px solid `#d9d9d9`;template/tinyvue/src/views/user/setting/index.vue (1)
20-48: Missing error handling forupdateInfoAPI call.If
userStore.updateInfo(newTemp)throws an error (e.g., network failure, server error), the promise rejection is unhandled. This could result in an uncaught promise rejection and a confusing user experience where nothing happens after clicking submit.🐛 Proposed fix
async function handleSubmit() { const data = setFormRef.value.setData() if (setFormRef.value.setFormValid()) { const newTemp = { department: data.filterOptions.department, job: data.filterOptions.position, employeeType: data.filterOptions.type, probationStart: getSimpleDate(data.filterOptions.date[0]), probationEnd: getSimpleDate(data.filterOptions.date[1]), probationDuration: data.filterOptions.during, protocolStart: getSimpleDate(data.filterOptions.startTime), protocolEnd: getSimpleDate(data.filterOptions.endTime), } - await userStore.updateInfo(newTemp) - - Modal.message({ - message: t('baseForm.form.submit.success'), - status: 'success', - }) - handleFormReset() + try { + await userStore.updateInfo(newTemp) + Modal.message({ + message: t('baseForm.form.submit.success'), + status: 'success', + }) + handleFormReset() + } + catch (error) { + Modal.message({ + message: t('baseForm.form.submit.error'), + status: 'error', + }) + } } else { Modal.message({ message: t('baseForm.form.submit.error'), status: 'error', }) } }template/tinyvue/src/views/list/search-table/index.vue (2)
152-159: Refresh the grid after successful deletion.After deleting an employee, the grid is not refreshed, leaving the deleted row visible until the user manually refreshes. Consider calling
handleRefresh()after successful deletion.Suggested fix
function handleDelete(id: string) { deleteEmployee(id).then(() => { Modal.message({ message: '已删除', status: 'success', }) + handleRefresh() }) }
152-159: Add error handling for the delete operation.If
deleteEmployeefails, the error is unhandled and the user receives no feedback.Suggested improvement
function handleDelete(id: string) { deleteEmployee(id).then(() => { Modal.message({ message: '已删除', status: 'success', }) + handleRefresh() + }).catch((error) => { + console.error('Failed to delete employee:', error) + Modal.message({ + message: '删除失败', + status: 'error', + }) }) }
🤖 Fix all issues with AI agents
In `@template/tinyvue/src/store/modules/user/index.ts`:
- Around line 92-95: The updateInfo method accepts Partial<UserInfo> but the
backend requires email and name, so either enforce those fields in the frontend
API call or merge missing fields from current state before sending; update the
updateInfo function to (a) require email and name in its parameter type (e.g.,
change Partial<UserInfo> to a type that includes email and name) or (b) build
the payload by merging this.info with data (e.g., payload = { ...this.info,
...data }) and pass that to updateUserInfo, then call this.setInfo(res.data) as
before; reference updateInfo, updateUserInfo, and UserInfo to locate where to
change.
In `@template/tinyvue/src/views/list/card-list/components/image.vue`:
- Around line 4-8: Destructuring src from defineProps makes it non-reactive so
url (the computed) won't update; instead call defineProps as a props object
(e.g., const props = defineProps<{ src?: string }>()) and reference props.src
inside the computed (or create a reactive ref via toRef(props, 'src')) and
handle the default value there so the computed URL uses the reactive prop;
update the computed that builds url to use props.src (or the toRef) rather than
the destructured src.
In `@template/tinyvue/src/views/list/search-table/components/EditDialog.vue`:
- Around line 101-106: The fetchEmployeeInfo function calls getEmployeeInfo
without error handling which can leave formModel inconsistent; wrap the await
getEmployeeInfo(id) call in a try/catch inside fetchEmployeeInfo, on success
populate formModel as before, on error log or surface the error (e.g.,
console.error or UI notification) and either leave formModel untouched or reset
its keys to safe defaults (e.g., ''), and return or rethrow an error indicator
so callers can react; update references to fetchEmployeeInfo, getEmployeeInfo,
and formModel accordingly.
In `@template/tinyvue/src/views/profile/detail/components/record-detail.vue`:
- Around line 20-23: The default page size in custPager (custPager.pageSize = 5)
isn't present in the component's page-sizes options, causing a mismatch; fix by
making them consistent: either change custPager.pageSize to one of the existing
options (e.g., 10) or add 5 to the page-sizes array used in the pagination
component (the template attribute `page-sizes`), then ensure the chosen value
appears in both custPager and the `page-sizes` list.
In `@template/tinyvue/src/views/user/info/components/info-tab.vue`:
- Line 78: The template has two elements sharing the same template ref created
by useTemplateRef('filterInfoBoxRef'), causing onClickOutside to only bind to
one element; change to separate refs (e.g., filterInfoBoxRefDesktop and
filterInfoBoxRefMobile) by creating two template refs via useTemplateRef and
bind each element to its corresponding ref, then update the
onClickOutside/handler setup to attach listeners to both refs (or conditionally
attach only to the currently rendered ref) so outside-click detection works for
both desktop and mobile containers.
🧹 Nitpick comments (23)
template/tinyvue/src/views/role/info/components/menu-drawer.vue (1)
82-83: Consider removing the empty style block.The empty
<style>block adds unnecessary boilerplate. It's better to add style blocks only when styles are actually needed, keeping the component leaner.♻️ Remove empty style block
- -<style lang="less" scoped></style>template/tinyvue/src/views/user/index.vue (1)
7-7: Consider removing the empty style block if it’s not intentional.Empty scoped styles add noise and make it harder to spot real styling changes later.
♻️ Proposed cleanup
-<style lang="less" scoped></style>template/tinyvue/src/views/list/index.vue (1)
7-7: Consider removing the empty style block if it’s not intentional.Keeps the SFC clean and avoids accumulating no-op styles.
♻️ Proposed cleanup
-<style scoped lang="less"></style>template/tinyvue/src/views/user/info/components/info-filterStatus.vue (2)
9-9: Consider adding explicit type annotation forcheckList.The
ref([])loses type information. Adding an explicit type improves type safety and IDE autocompletion.-const checkList = ref([]) +const checkList = ref<string[]>([])
42-43: Remove empty style block.The empty
<style>block serves no purpose and should be removed for cleaner code.-<style scoped lang="less"> -</style>template/tinyvue/src/views/user/info/components/info-table.vue (1)
8-8: Consider using a specific type instead ofany[].Using
any[]loses type safety. Define or import an interface for the table data structure to improve maintainability and catch type errors at compile time.-defineProps<{ tableData: any[] }>() +interface TableRow { + name: string + time: string + type: string + status: string +} +defineProps<{ tableData: TableRow[] }>()template/tinyvue/src/views/user/info/components/info-filterStartTime.vue (1)
34-34: Remove empty style block.Same as other filter components - the empty style block should be removed.
-<style scoped lang="less"></style>template/tinyvue/src/views/not-found/index.vue (1)
26-27: Remove empty style block.-<style scoped lang="less"> -</style>template/tinyvue/src/views/user/info/components/info-filter.vue (1)
35-55: Consider refactoring ternary operators used for control flow.Using ternary operators (
?:) for side effects (showing modals vs. setting state) is unconventional and harder to read thanif/elseblocks.♻️ Suggested refactor for readability
function submit() { if (props.activeName === '1') { - userStore.startTime === '' - || userStore.endTime === '' - || userStore.filterStatus?.length === 0 - || userStore.filterType?.length === 0 - ? Modal.message({ - message: t('userInfo.filter.all'), - status: 'error', - }) - : userStore.setInfo({ submit: true, sort: undefined }) + const isInvalid = userStore.startTime === '' + || userStore.endTime === '' + || userStore.filterStatus?.length === 0 + || userStore.filterType?.length === 0 + + if (isInvalid) { + Modal.message({ message: t('userInfo.filter.all'), status: 'error' }) + } else { + userStore.setInfo({ submit: true, sort: undefined }) + } } else { - userStore.filterStatus?.length === 0 || userStore.filterType?.length === 0 - ? Modal.message({ - message: t('userInfo.filter.all'), - status: 'error', - }) - : userStore.setInfo({ submit: true, sort: undefined }) + const isInvalid = userStore.filterStatus?.length === 0 || userStore.filterType?.length === 0 + + if (isInvalid) { + Modal.message({ message: t('userInfo.filter.all'), status: 'error' }) + } else { + userStore.setInfo({ submit: true, sort: undefined }) + } } }template/tinyvue/src/views/user/info/index.vue (1)
2-5: Good refactor to async components.The use of
defineAsyncComponentfor lazy-loading is a valid optimization. However, consider adding loading and error components for better UX, especially if these components are heavy or if the network is slow.💡 Optional: Add loading/error handling for async components
-const Headtop = defineAsyncComponent(() => import('../../form/step/components/head.vue')) -const Infotab = defineAsyncComponent(() => import('./components/info-tab.vue')) +const Headtop = defineAsyncComponent({ + loader: () => import('../../form/step/components/head.vue'), + loadingComponent: () => h('div', 'Loading...'), + delay: 200, +}) +const Infotab = defineAsyncComponent({ + loader: () => import('./components/info-tab.vue'), + loadingComponent: () => h('div', 'Loading...'), + delay: 200, +})template/tinyvue/src/views/user/setting/components/set-from.vue (1)
200-201: Empty style block is unnecessary.The empty
<style>block adds no value and can be removed.♻️ Proposed fix
- -<style lang="less" scoped></style>template/tinyvue/src/api/profile.ts (1)
12-13: Consider adding return type annotation for better type safety.The
getDetailDatafunction could benefit from an explicit return type annotation to ensure consumers know what to expect.💡 Proposed improvement
-export function getDetailData() { +export function getDetailData(): Promise<AxiosResponse<DetailTableData[]>> { return axios.get(`${import.meta.env.VITE_MOCK_SERVER_HOST}/api/detail/getdata`) }Note: You may need to import
AxiosResponsefrom axios and verify the actual response structure matchesDetailTableData[].template/tinyvue/src/views/user/info/components/info-chart.vue (1)
7-7: Consider adding a proper type forchartDataprop.Using
any[]loses type safety. Define an interface for the chart data structure to improve maintainability and catch type errors at compile time.💡 Proposed improvement
interface ChartItem { title: string value: number list?: Array<{ sort: number type: string len: number pid: 'A' | 'B' | 'C' | 'D' status: string }> } defineProps<{ chartData: ChartItem[] }>()template/tinyvue/src/views/profile/detail/index.vue (3)
14-14: Avoidanytype for the loading ref.The
ref<any>()loses type safety. Consider using the actual return type fromLoading.serviceor at least a more specific type likeref<{ close: () => void } | null>(null).Suggested improvement
-const loading = ref<any>() +const loading = ref<{ close: () => void } | null>(null)
26-32: Add error handling for the API call.If
getDetailData()fails, the error is silently swallowed and the user receives no feedback. Consider adding a catch block with user notification.Suggested improvement
try { const { data } = await getDetailData() tableData.value = data.tableData } + catch (error) { + console.error('Failed to fetch detail data:', error) + // Consider showing user-facing error notification + } finally { loading.value.close() }
7-12: Consider adding loading/error states for async components.When using
defineAsyncComponent, you can provideloadingComponentanderrorComponentoptions to handle loading states and errors gracefully, improving the user experience.template/tinyvue/src/views/profile/detail/components/record-detail.vue (1)
33-37: Simplify the pagination slice calculation.The
startindex is computed twice. Consider reusing it:Suggested improvement
const listData = computed(() => { const start = (custPager.value.currentPage - 1) * custPager.value.pageSize - const end = (custPager.value.currentPage - 1) * custPager.value.pageSize + custPager.value.pageSize - return tableData.slice(start, end) + return tableData.slice(start, start + custPager.value.pageSize) })template/tinyvue/src/views/list/card-list/index.vue (2)
62-63: Add null check before observing pagerRef.
pagerRef.valuecould be null if called before the component is mounted. WhilehandleResizePageris called inonMounted, it's good practice to add a guard.Suggested improvement
observer = new ResizeObserver(handleResize) - observer.observe(pagerRef.value.$el) + if (pagerRef.value?.$el) { + observer.observe(pagerRef.value.$el) + }
91-93: Consider adding type annotations for refs.The
cardsandcardLoadingStaterefs lack type annotations. Adding types improves code clarity and catches potential errors.Suggested improvement
-const cards = ref([]) +const cards = ref<any[]>([]) // Replace 'any' with actual card type from API -const cardLoadingState = ref(null) +const cardLoadingState = ref<{ close: () => void } | null>(null)template/tinyvue/src/views/list/search-table/components/EditDialog.vue (3)
83-99: Handle validation rejection and consider using async/await.When form validation fails, the rejection is silently ignored. Consider handling validation errors explicitly and refactoring to async/await for better readability.
Suggested improvement
-function handleUpdateSubmit() { - localeForm.value.validate().then(() => { - updateEmployeeInfo(formModel).then(() => { +async function handleUpdateSubmit() { + try { + await localeForm.value.validate() + await updateEmployeeInfo(formModel) + Modal.message({ + message: '更新成功', + status: 'success', + }) + onRefresh?.() + visible.value = false + } + catch (error) { + if (error !== false) { // validation returns false on failure Modal.message({ - message: '更新成功', - status: 'success', + message: '更新失败', + status: 'error', }) - onRefresh?.() - visible.value = false - }).catch(() => { - Modal.message({ - message: '更新失败', - status: 'error', - }) - }) - }) + } + } }
43-79: Use i18n for all user-facing strings.Several strings are hardcoded in Chinese (department levels, departments, roles, user names) while other labels use
$t()for internationalization. For consistency and proper localization support, consider using i18n keys for all dropdown options and messages.Examples of hardcoded strings:
- Lines 44-46:
'一级','二级','三级'- Lines 50-51:
'公共服务部','计算管理部'- Lines 70-72:
'前端','后端','测试'- Line 87, 94:
'更新成功','更新失败'
108-115: Consider resetting form data when dialog opens.When the dialog opens for a different employee, the previous employee's data may be briefly visible before the new data loads. Consider resetting
formModelbefore fetching new data.Suggested improvement
watch( visible, (newVal) => { if (newVal && employeeId) { + // Reset form before fetching new data + Object.keys(formModel).forEach((key) => { + formModel[key] = '' + }) fetchEmployeeInfo(employeeId) } }, )template/tinyvue/src/views/list/search-table/index.vue (1)
305-305: Use i18n for the delete confirmation message.The confirmation title
"确定要删除此用户吗？"is hardcoded in Chinese. For consistency with the rest of the codebase which uses$t(), consider using an i18n key.-<TinyPopconfirm title="确定要删除此用户吗？" type="info" trigger="click" `@confirm`="handleDelete(data.row.id)"> +<TinyPopconfirm :title="$t('searchTable.deleteConfirm')" type="info" trigger="click" `@confirm`="handleDelete(data.row.id)">
1 participant
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Improvements
Style
✏️ Tip: You can customize this high-level summary in your review settings.