-
-
Notifications
You must be signed in to change notification settings - Fork 53.5k
feat: FloatButton support semantic structure & ConfigProvider support passing related props #54145
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
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
👁 Visual Regression Report for PR #54145 Failed ❌
Warning There are more diffs not shown in the table. Please check the Full Report for details. Important There are 33 diffs found in this PR: 🔄
|
size-limit report 📦
|
More templates
commit: |
Deploying ant-design with
|
Latest commit: |
245649a
|
Status: | ✅ Deploy successful! |
Preview URL: | https://95044e2e.ant-design.pages.dev |
Branch Preview URL: | https://float-btn.ant-design.pages.dev |
Bundle ReportChanges will decrease total bundle size by 7.8MB (-67.53%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: antd.min-array-pushAssets Changed:
Files in
Files in
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #54145 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 789 789
Lines 14371 14379 +8
Branches 3786 3789 +3
=========================================
+ Hits 14371 14379 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR adds CSS variable support and semantic structure to the FloatButton components, and enables passing semantic props via ConfigProvider.
- Introduce
genCssVar
utility to standardize CSS variable generation. - Refactor
FloatButton
andFloatButtonGroup
to use the sharedButton
component and merge semanticclassNames
/styles
. - Extend
ConfigProvider
to acceptfloatButton
andfloatButtonGroup
configuration, and update docs/demos to use the newcontent
prop.
Reviewed Changes
Copilot reviewed 34 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
components/theme/util/genStyleUtils.ts | Add genCssVar helper for building CSS variable names |
components/steps/style/index.ts | Introduce CSS vars for Steps; leave TODO for using genCssVar |
components/float-button/style/group.ts | New group styles using CSS vars and semantic tokens |
components/float-button/style/button.ts | New float-button styles with CSS variable hooks |
components/float-button/FloatButton.tsx | Refactor to Button component, semantic classNames /styles |
components/float-button/FloatButtonGroup.tsx | Refactor group, context, and merge semantic props |
components/config-provider/context.ts | Add floatButton and floatButtonGroup config types |
components/float-button/demo & docs updates | Replace description with content ; add semantic DOM demos |
@@ -41,3 +41,12 @@ export const { genStyleHooks, genComponentStyleHook, genSubStyleComponent } = ge | |||
getCommonStyle: genCommonStyle, | |||
getCompUnitless: (() => unitless) as GetCompUnitless<ComponentTokenMap, AliasToken>, | |||
}); | |||
|
|||
export const genCssVar = (antCls: string, componentAbbr: string) => { | |||
const cssPrefix = `--${antCls.replace('.', '')}-${componentAbbr}-`; |
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.
[nitpick] Use a global replace (e.g. antCls.replace(/\./g, '')
) to ensure all dots are removed, not just the first occurrence.
const cssPrefix = `--${antCls.replace('.', '')}-${componentAbbr}-`; | |
const cssPrefix = `--${antCls.replace(/\./g, '')}-${componentAbbr}-`; |
Copilot uses AI. Check for mistakes.
content: `${prefixCls}-content`, | ||
}), | ||
[prefixCls], | ||
); |
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.
useMergeSemantic
omits ConfigProvider
context-level classNames
and styles
, so provider-level semantic settings for FloatButton won't be applied. Consider adding contextClassNames and contextStyles back into the merge.
Copilot uses AI. Check for mistakes.
|
||
const wrapperCls = classNames(hashId, `${groupPrefixCls}-wrap`); | ||
// ========================== Placement ========================== | ||
const mergedPlacement = ['top', 'left', 'right', 'bottom'].includes(placement!) |
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.
[nitpick] Using non-null assertion on placement
may mask undefined cases. Consider defaulting placement
earlier or using a local variable safely instead of placement!
.
const mergedPlacement = ['top', 'left', 'right', 'bottom'].includes(placement!) | |
const mergedPlacement = ['top', 'left', 'right', 'bottom'].includes(placement) |
Copilot uses AI. Check for mistakes.
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log