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: picker full token #35615

Merged
merged 7 commits into from
May 20, 2022
Merged

refactor: picker full token #35615

merged 7 commits into from
May 20, 2022

Conversation

MadCcc
Copy link
Member

@MadCcc MadCcc commented May 18, 2022

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English -
🇨🇳 Chinese -

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@github-actions
Copy link
Contributor

github-actions bot commented May 18, 2022

@github-actions
Copy link
Contributor

github-actions bot commented May 18, 2022

Size Change: +294 B (0%)

Total Size: 888 kB

Filename Size Change
./dist/antd-with-locales.min.js 455 kB +83 B (0%)
./dist/antd.min.js 402 kB +211 B (0%)
ℹ️ View Unchanged
Filename Size
./dist/antd.compact.min.css 7.78 kB
./dist/antd.dark.min.css 7.83 kB
./dist/antd.min.css 7.8 kB
./dist/antd.variable.min.css 8.19 kB

compressed-size-action

@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

Merging #35615 (756efbb) into next (651dae1) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              next    #35615   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          465       465           
  Lines         8470      8470           
  Branches      2401      2401           
=========================================
  Hits          8470      8470           
Impacted Files Coverage Δ
components/_util/theme/util/alias.ts 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 651dae1...756efbb. Read the comment docs.

@MadCcc MadCcc changed the title [WIP] refactor: picker full token refactor: picker full token May 19, 2022
@@ -92,6 +92,7 @@ export default function formatToken(derivativeToken: RawMergedToken): AliasToken
colorSplit: 'rgba(0, 0, 0, 0.06)',
controlItemBgActive: primaryColors[0],
fontWeightStrong: 600,
fontWeight: 400,
Copy link
Member

Choose a reason for hiding this comment

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

感觉 fontWeight 400 像是一个固定值。感觉直接下面用到 400 的使用 normal 代替?

截屏2022-05-19 下午5 17 52

yearSelectWidth: number;
monthSelectWidth: number;
miniContentHeight: number;
thHeight: number;
Copy link
Member

Choose a reason for hiding this comment

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

不够语义化,这个和 dom 强耦合了

calendarFullPanelBg: string;
calendarItemActiveBg: string;
export interface ComponentToken {
yearSelectWidth: number;
Copy link
Member

Choose a reason for hiding this comment

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

这应该是 yearPanelWidth,下同

Copy link
Member Author

Choose a reason for hiding this comment

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

这个还真是 select 的宽度。。

Copy link
Member

Choose a reason for hiding this comment

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

2333,好吧~

Copy link
Member

Choose a reason for hiding this comment

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

controlXXXWidth

export interface ComponentToken {
yearSelectWidth: number;
monthSelectWidth: number;
miniContentHeight: number;
Copy link
Member

Choose a reason for hiding this comment

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

直接 panelHeight

monthSelectWidth: number;
miniContentHeight: number;
thHeight: number;
dateValueHeight: number;
Copy link
Member

Choose a reason for hiding this comment

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

做成 PickerToken 然后用 controlHeightSM

calendarItemActiveBg: token.controlItemBgActive,
dateValueHeight: token.controlHeightSM,
weekHeight: token.controlHeightSM * 0.75,
dateContentHeight: (token.fontSizeSM * token.lineHeights[0] + token.marginXS) * 3 + 2,
Copy link
Member

Choose a reason for hiding this comment

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

这个 lineHeight 可以抽一个 lineHeightSM 和 fontSizeSM 对应了。我们应该不能直接访问 lineHeights 才对

Copy link
Member

Choose a reason for hiding this comment

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

+2 这个改成 border * 2

borderBlockEnd: 0,
borderInlineStart: 1.5, // FIXME: v4 magic
borderInlineEnd: 0,
borderBlockStartWidth: 1.5, // FIXME: v4 magic
Copy link
Member

Choose a reason for hiding this comment

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

1.5 太多了,抽到 PickerToken 里

color: token.colorText,
lineHeight: '30px', // FIXME: v4 magic string
lineHeight: `${token.pickerPanelCellHeight + token.pickerCellPaddingVertical * 2}px`,
Copy link
Member

Choose a reason for hiding this comment

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

话说 th 是不是直接可以 vertical align 了?

@MadCcc MadCcc merged commit d560df6 into next May 20, 2022
@MadCcc MadCcc deleted the refactor/picker-full-token branch May 20, 2022 06:33
@MadCcc MadCcc mentioned this pull request May 20, 2022
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.

2 participants