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

New icons: plus-one and minus-one #296

Merged
merged 1 commit into from
Oct 19, 2020

Conversation

a26620236
Copy link
Contributor

@a26620236 a26620236 commented Oct 12, 2020

Purpose

  • 新增兩個改變數量的按鈕,plus-one/minus-one,用來改變交易明細列印設定裡面的預設張數

  • plus-one:

image

  • minus-one:

image

@kyoyadmoon
Copy link
Member

kyoyadmoon commented Oct 12, 2020

看起來顏色好像怪怪的 (?
感覺是 fill 沒改到的關係,白底的地方也要記得拿掉喔
另外好奇這裡的 border 跟中心的會需要不一樣嗎?
如果兩個都要可以客製化顏色,就要再加一個 color prop 了
image

return (
<svg width="1em" height="1em" viewBox="0 0 30 30" fill="none" {...props}>
<circle cx={15} cy={15} r={14.5} fill="#fff" stroke="#B0B0B0" />
<path fill="#717171" d="M10 14h11v2H10z" />
Copy link
Member

Choose a reason for hiding this comment

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

這裡是不是要改成

fill="currentColor"

Copy link
Contributor

Choose a reason for hiding this comment

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

@kyoyadmoon 有沒有想把 Icon story 的 background 改成有顏色的底!

Copy link
Member

Choose a reason for hiding this comment

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

這件事印象中之前 @chenesan 在改的時候好像有討論過
忘記後來為什麼沒做了

Copy link
Contributor

@zhusee2 zhusee2 left a comment

Choose a reason for hiding this comment

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

others LGTM

packages/core/src/icons/components/MinusOne.js Outdated Show resolved Hide resolved
export default function SvgMinusOne(props) {
return (
<svg width="1em" height="1em" viewBox="0 0 30 30" fill="none" {...props}>
<circle cx={15} cy={15} r={14.5} fill="#fff" stroke="#B0B0B0" />
Copy link
Member

@kyoyadmoon kyoyadmoon Oct 12, 2020

Choose a reason for hiding this comment

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

這裡要把 fill 拿掉,不然如果背景是其他顏色,就會像下面的圖 😂
circle stroke 的顏色可以找設計確認一下是不是固定的

Suggested change
<circle cx={15} cy={15} r={14.5} fill="#fff" stroke="#B0B0B0" />
<circle cx={15} cy={15} r={14.5} fill="transparent" stroke="#B0B0B0" />

image

@a26620236 a26620236 force-pushed the feature/add-icon-plus-one-and-minus-one branch from 2d0b799 to 7fe2f01 Compare October 12, 2020 09:33
export default function SvgMinusOne(props) {
return (
<svg width="1em" height="1em" viewBox="0 0 30 30" fill="none" {...props}>
<circle cx={15} cy={15} r={14.5} fill="transparent" stroke="#B0B0B0" />
Copy link
Contributor

@benny0642 benny0642 Oct 13, 2020

Choose a reason for hiding this comment

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

好奇,如果變色的話, circle 的 stroke 要跟著變嗎?

譬如說,在 error status 的情況下,circle stroke 依然維持 #B0B0B0 還是 red?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

最後設計師決定不填滿了,只有改變 stroke 。

@a26620236 a26620236 force-pushed the feature/add-icon-plus-one-and-minus-one branch from 734ab0f to 64ce3a8 Compare October 16, 2020 09:35
Copy link
Member

@kyoyadmoon kyoyadmoon left a comment

Choose a reason for hiding this comment

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

@a26620236 a26620236 merged commit 317fe6b into develop Oct 19, 2020
@a26620236 a26620236 deleted the feature/add-icon-plus-one-and-minus-one branch October 19, 2020 02:15
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.

6 participants