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.

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

refactor: 优化Drawer滚动 & 优化VirtualList参数配置 #330

Merged
merged 2 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions components/drawer/drawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
} from 'vue';
import { isNumber } from 'lodash-es';
import getPrefixCls from '../_util/getPrefixCls';
import FButton from '../button/button';
import FButton from '../button';
import FScrollbar from '../scrollbar';
import { CloseOutlined } from '../icon';
import PopupManager from '../_util/popupManager';
import useLockScreen from '../_util/use/useLockScreen';
Expand Down Expand Up @@ -215,9 +216,12 @@ const Drawer = defineComponent({
onClick={(event) => event.stopPropagation()}
>
{getHeader()}
<div class={`${prefixCls}-body`}>
<FScrollbar
class={`${prefixCls}-body-wrapper`}
containerClass={`${prefixCls}-body-container`}
>
{ctx.slots.default?.()}
</div>
</FScrollbar>
{getFooter()}
</div>
</div>

Choose a reason for hiding this comment

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

这是一个 Vue 组件的代码补丁,需要对其进行简短的代码审查,包括可能存在的错误风险和改进意见。建议如下:

  • 建议修改导入语句中的默认按钮组件 FButton 为 '../button',因为在后面的使用中,FButton 已经被直接引入过一次。
  • 建议为组件添加更多注释。
  • 建议在适当的位置添加代码分隔符,增强可读性。
  • 对于新增的 FScrollbar 组件,建议添加更多测试用例来完善单元测试,以保证其稳定性。

Expand Down
15 changes: 9 additions & 6 deletions components/drawer/style/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
.fixed-full();
display: flex;
}

&-wrapper {
position: relative;
display: flex;
Expand All @@ -30,7 +30,7 @@
border-radius: var(--f-border-radius-base);
box-shadow: @shadow-down;
}

&-header {
position: relative;
display: flex;
Expand All @@ -47,10 +47,13 @@
}
}

&-body {
&-body-wrapper {
flex: 1;
}

&-body-container {
box-sizing: border-box;
padding: @padding-md calc(@padding-md + @padding-xs);
overflow: auto;
color: var(--f-sub-head-color);
font-size: var(--f-font-size-base);
}
Expand Down Expand Up @@ -78,7 +81,7 @@
&-mask-fade-enter-active {
transition: opacity @animation-duration-slow @ease-base-in;
}

&-mask-fade-leave-to,
&-mask-fade-enter-from {
opacity: 0;
Expand Down Expand Up @@ -155,4 +158,4 @@
}
}

}
}

Choose a reason for hiding this comment

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

这段代码补丁中有以下几点可以改进:

  • 第一处改动是在样式表中删去了一个空行,这个改动并不影响样式表的正确性。
  • 第二处改动是为了增加代码可读性,建议给 &-wrapper&-body-container 类名添加注释,如 /* 容器 */
  • 第三处改动是将原来的 &-body 改为了 &-body-wrapper,同时引入了一个新的 &-body-container 元素。可以推断出,这个改动是为了更好地控制内容的滚动,但在代码框架中缺少了新元素的定义,因此建议添加 &-body-container 的具体样式规则。
  • 最后,代码最后一行缺少换行符号。

没有发现明显的潜在问题或危险,建议在新增代码前先对整个样式表进行语法检查和格式化操作。

8 changes: 6 additions & 2 deletions components/virtual-list/listItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,12 @@ export const FVirtualListItem = defineComponent({
},
render() {
const { index, source, $slots } = this;

const vNode = getFirstValidNode($slots.default?.({ index, source }));
const vNode = getFirstValidNode(
$slots.default?.({ index, source }) ?? [],
);
if (!vNode) {
return;
}
return cloneVNode(
vNode,
{
Expand Down
1 change: 1 addition & 0 deletions components/virtual-list/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const virtualProps = {
dataSources: {
type: Array as PropType<DataSource[]>,
required: true,
default: (): DataSource[] => [],
},
keeps: {
type: Number,

Choose a reason for hiding this comment

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

以下是对此代码补丁的简要代码审查:

在这个代码补丁中,我们看到一些更改。具体来说,在 dataSources 属性中添加了一个默认值。

从代码来看,这似乎是合理的更改。这将确保即使没有传递数据源属性,代码也可以正常运行,并且不会抛出未定义的错误。另外,这个默认值似乎是由一个函数提供的,这意味着它可能取决于组件当前的状态,而不是简单的硬编码。

总体而言,这似乎是一个推荐的改进,因为它提高了代码的鲁棒性,降低了错误风险。

Expand Down
2 changes: 1 addition & 1 deletion docs/.vitepress/components/drawer/common.vue
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
:footer="true"
@ok="show[4] = false"
>
<div>我是内容...</div>
<div style="height: 1000px">我是内容...</div>
<div>我是内容...</div>
<div>我是内容...</div>
</FDrawer>

Choose a reason for hiding this comment

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

这段代码看起来只是在修改了一个组件的样式,将其中一个 <div> 元素的高度设置为了 1000 像素,这样可能会影响页面的排版和渲染速度。如果这种更改不是必要的,可以考虑删除或者优化这个修改,并且需要测试这个修改是否引入了新的 bug 或风险。另外,建议在代码中添加注释以使其更易于阅读和理解。

Expand Down