Skip to content

type(list): If loadingText is set to null, hidden the loading comp #13440

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yoyo837
Copy link
Contributor

@yoyo837 yoyo837 commented Apr 16, 2025

允许 loadingText 设置为null, 不显示loading效果

@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.67%. Comparing base (ec5b45b) to head (fbbc7fd).
Report is 97 commits behind head on main.

Files with missing lines Patch % Lines
packages/vant/src/list/List.tsx 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13440      +/-   ##
==========================================
+ Coverage   89.60%   89.67%   +0.07%     
==========================================
  Files         257      257              
  Lines        7013     7034      +21     
  Branches     1736     1742       +6     
==========================================
+ Hits         6284     6308      +24     
+ Misses        384      383       -1     
+ Partials      345      343       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

if (slots.loading) {
return <div class={bem('loading')}>{slots.loading()}</div>;
}
const text = props.loadingText ?? t('loading');
Copy link
Member

Choose a reason for hiding this comment

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

建议允许传入 null,并在值为 null 的时候不展示 loading,这样可以避免 break 现有的项目

Copy link
Member

Choose a reason for hiding this comment

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

参考

indeterminate: {
type: Boolean as PropType<boolean | null>,
default: null,
},
}),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我之所以这样写,是因为finished-text=""能达到不显示加载完毕的提示,想着他们俩应该尽量统一,避免认知错乱。
所以,仍然按上面要求的改吗?佬。

Copy link
Member

Choose a reason for hiding this comment

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

抱歉回复晚了,我还是倾向于尽可能不破坏现有的行为,所以新增一个 null 是比较合适的选择

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@yoyo837 yoyo837 force-pushed the allow-loadingtext-empty branch from 094f197 to fbbc7fd Compare May 21, 2025 03:08
@yoyo837 yoyo837 changed the title type(list):If loadingText is set to an empty string, hidden the loading comp type(list):If loadingText is set to null, hidden the loading comp May 21, 2025
@yoyo837 yoyo837 changed the title type(list):If loadingText is set to null, hidden the loading comp type(list): If loadingText is set to null, hidden the loading comp May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants