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

style: optimization dropdown menu style #21768

Merged
merged 4 commits into from
Mar 4, 2020
Merged

style: optimization dropdown menu style #21768

merged 4 commits into from
Mar 4, 2020

Conversation

xrkffgg
Copy link
Member

@xrkffgg xrkffgg commented Mar 2, 2020 •
edited by afc163
Loading

🤔 This is a ...

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

🔗 Related issue link

close #21767

💡 Background and solution

  • 根据 Issue ，将right 调整为 2，这样总的为 22px ，距离左侧文字 4px

📝 Changelog

Language Changelog
🇺🇸 English Fix Dropdown menu arrow position.
🇨🇳 Chinese 修复 Dropdown 菜单里箭头图标的位置。

☑️ Self Check before Merge

  • 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

View rendered components/dropdown/demo/overlay-visible.md
View rendered components/dropdown/index.zh-CN.md

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Mar 2, 2020 •
edited
Loading

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 2, 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 7bbaa42:

Sandbox Source
antd reproduction template Configuration

@zombieJ
Copy link
Member

zombieJ commented Mar 2, 2020 •
edited
Loading

应该用计算求值，不能变量转 magic number ……

@xrkffgg
Copy link
Member Author

xrkffgg commented Mar 2, 2020

@padding-xs 是 变量里 定义最小的了。。。

那这样，就不动 right
修改 padding-right 可 好 ? 因为它本身定义的就是使用的 数字 26px

@codecov
Copy link

codecov bot commented Mar 2, 2020 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #21768   +/-   ##
=======================================
  Coverage   97.88%   97.88%           
=======================================
  Files         305      305           
  Lines        7003     7003           
  Branches     1889     1889           
=======================================
  Hits         6855     6855           
  Misses        148      148

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 27e48d3...7bbaa42. Read the comment docs.

@xrkffgg
Copy link
Member Author

xrkffgg commented Mar 2, 2020

@zombieJ
这里的都用了变量

样式本身的 padding + 图片的 right + 图标的大小

@yoyo837
Copy link
Contributor

yoyo837 commented Mar 2, 2020

image

图标本身有个margin-right, 是不是可以把包裹元素的right改为0 ？又或者是去掉图标的margin-right?

@xrkffgg
Copy link
Member Author

xrkffgg commented Mar 3, 2020

图标的 margin-right 最好别动，因为左边的图标 也要用到这个

我觉得可以把 right 设为 0，然后调整 padding-right

@xrkffgg
Copy link
Member Author

xrkffgg commented Mar 3, 2020

@yoyo837 你觉得如何？

@yoyo837
Copy link
Contributor

yoyo837 commented Mar 3, 2020

两种都是修改dropdown的样式，影响只是dropdown，而现在就是在改dropdown, 所以我倾向把icon的margin 均摊到两边。

@afc163
Copy link
Member

afc163 commented Mar 4, 2020

@yoyo837 @zombieJ @xrkffgg 这个完成了么

afc163
afc163 reviewed Mar 4, 2020
View reviewed changes
components/dropdown/style/index.less
@@ -189,6 +189,7 @@
}

&-icon {
margin-right: 0;
Copy link
Member

Choose a reason for hiding this comment

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

这里重置一下就行了。

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+8 16px 距离右侧边距有点太大了

@afc163 afc163 merged commit 1556582 into ant-design:master Mar 4, 2020
@xrkffgg xrkffgg deleted the fix-dropdown branch March 4, 2020 03:55
@zombieJ zombieJ mentioned this pull request Mar 4, 2020
Merged
10 tasks
@snyk-bot snyk-bot mentioned this pull request Mar 10, 2020
Merged
@xrkffgg
Copy link
Member Author

xrkffgg commented Mar 11, 2020 •
edited
Loading

@afc163 大佬，你改了一下，有点小问题呀

image

menu 被图标覆盖，没显示全。

如果要保持 submenu-arrow 这个不变

&-submenu-title 要调整
padding-right: @control-padding-horizontal + @padding-xs + @font-size-sm;

如果 submenu-arrow 为 right 为 0
&-submenu-title 就可以去掉 @padding-xs 了
padding-right: @control-padding-horizontal + @font-size-sm;

这两种就是图标和边界的距离不一样，感觉这两种距离看着 都可以。

@yoyo837
Copy link
Contributor

yoyo837 commented Mar 11, 2020

再发补充PR

@xrkffgg
Copy link
Member Author

xrkffgg commented Mar 11, 2020

补充 PR 和 平时的 PR 一样吗

@xrkffgg xrkffgg mentioned this pull request Mar 11, 2020
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afc163 afc163 afc163 left review comments

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

Successfully merging this pull request may close these issues.

bug: dropdown menu width style
5 participants
@xrkffgg @ant-design-bot @zombieJ @yoyo837 @afc163