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.

Sign up for GitHub

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

Jump to bottom

fix: TimePicker panel align #25019

Merged
merged 4 commits into from
Jun 18, 2020
Merged

fix: TimePicker panel align #25019

merged 4 commits into from
Jun 18, 2020

Conversation

xrkffgg
Copy link
Member

@xrkffgg xrkffgg commented Jun 16, 2020 •
edited
Loading

[中文版模板 / Chinese template]

🤔 This is a ...

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

🔗 Related issue link

💡 Background and solution

  • DataPicker/TimePicker 时间下拉选，当选择最后一个时，会出现水平方向不对齐。
# Pic
Before ❌ image
After ✅ image

📝 Changelog

Language Changelog
🇺🇸 English Fix DataPicker/TimePicker time dropdown alignment problem.
🇨🇳 Chinese 修复 DataPicker/TimePicker 时间下拉选对齐问题。

☑️ 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

xrkffgg
xrkffgg commented Jun 16, 2020
View reviewed changes
components/date-picker/style/panel.less Outdated
@@ -597,7 +597,7 @@
flex: 1 0 auto;
width: 56px;
margin: 0;
padding: 0 0 194px 0;
padding: 0 0 224px - @picker-time-panel-cell-height 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

593 行使用了 height: 224px，这里也用了 固定的 px，其实 224 px 是 8 倍 的 @picker-time-panel-cell-height，表示 一个数列最多展示 8 条，要不要把这个提成个变量？

Copy link
Member Author

Choose a reason for hiding this comment

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

对了，如果要提成变量的话，是提 8 还是 提 224，我觉得 8 ，这样都是倍数，224 的话，别人改成 225 226 显示的就不是整的，这样还是会出现 不对齐的问题。

如果不提变量，也行，默认 8 倍的

Copy link
Member Author

Choose a reason for hiding this comment

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

再顺便说一句，224px - @picker-time-panel-cell-height 是 196

@xrkffgg xrkffgg requested review from zombieJ and afc163 June 16, 2020 03:54
@ant-design-bot
Copy link
Contributor

ant-design-bot commented Jun 16, 2020 •
edited
Loading

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Jun 16, 2020 •
edited
Loading

@afc163
Copy link
Member

afc163 commented Jun 16, 2020

在 Firefox 下这个 padding 直接不生效：

image

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 16, 2020

我试试，我用谷歌

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 16, 2020 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit b58b2ee:

Sandbox Source
antd reproduction template Configuration

@codecov
Copy link

codecov bot commented Jun 16, 2020 •
edited
Loading

Codecov Report

Merging #25019 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25019   +/-   ##
=======================================
  Coverage   99.38%   99.38%           
=======================================
  Files         365      365           
  Lines        7294     7294           
  Branches     2030     1989   -41     
=======================================
  Hits         7249     7249           
  Misses         45       45

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 a5f3ac8...b58b2ee. Read the comment docs.

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 16, 2020

我这 Firefox 76.0.1 也有问题

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 16, 2020

大佬，换了种写法。兼容性好点。
@afc163

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 16, 2020

https://stackoverflow.com/questions/48687129/padding-bottom-not-working-in-firefox-ie11

@zombieJ
Copy link
Member

zombieJ commented Jun 16, 2020

截屏2020-06-16 下午6 43 30

Chrome 下还是多了一点点

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 16, 2020

image

我这里看着没问题啊。。。。

@zombieJ
Copy link
Member

zombieJ commented Jun 16, 2020

中间那个往下滚应该会出一点

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 16, 2020

嗯，正常选是没事，但是继续滚动是会出现。
最开始时候，我是没加那个 2px 的，height 是 196px 的时候，TimePicker 是都没问题的。
但是我看这个时候 DatePicker 里的时间选项有明显的对齐问题，然后我才改成了 198px，DatePicker 是正常了，下滚也不会出现，但是又出现 你发现的 TimePicker 下滚会多出来一点的问题。

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 16, 2020

DatePicker 里多出来 2px 就很奇怪

image

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 17, 2020

验证 DatePicker 中 time 选择

https://ant.design/components/date-picker-cn/#components-date-picker-demo-presetted-ranges

验证 TimePicker 中 time 选择

https://ant.design/components/time-picker-cn/#components-time-picker-demo-basic

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 17, 2020

@zombieJ 这次两种情况都包上了。

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 17, 2020

PTAL

@xrkffgg
Copy link
Member Author

xrkffgg commented Jun 18, 2020

/rebase

@afc163 afc163 merged commit 7f2d8a4 into master Jun 18, 2020
@afc163 afc163 deleted the fix-timepicker-align branch June 18, 2020 02:54
@pr-triage pr-triage bot added the PR: merged label Jun 18, 2020
@afc163 afc163 mentioned this pull request Jun 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zombieJ zombieJ Awaiting requested review from zombieJ

@afc163 afc163 Awaiting requested review from afc163

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xrkffgg @ant-design-bot @afc163 @zombieJ