Skip to content

button border width grow outside the container #3221

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

Merged
merged 4 commits into from
Aug 22, 2024
Merged

button border width grow outside the container #3221

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a266e0
button border width grow outside the container
adids1221 Aug 21, 2024
1bb87dc
Merge branch 'master' into fix/button_border_width_consistent_style
adids1221 Aug 22, 2024
600f67d
removed forEach for round button
adids1221 Aug 22, 2024
d1cf2e8
update snapshot
adids1221 Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions demo/src/screens/__tests__/__snapshots__/TextFieldScreen.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3521,8 +3521,8 @@ exports[`TextField Screen renders screen 1`] = `
"marginLeft": 20,
"minWidth": 66,
"opacity": 1,
"paddingHorizontal": 10,
"paddingVertical": 2,
"paddingHorizontal": 11,
"paddingVertical": 3,
}
}
>
Expand Down
42 changes: 21 additions & 21 deletions src/components/button/__tests__/__snapshots__/index.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,8 @@ exports[`Button backgroundColor should return undefined if this button is outlin
"justifyContent": "center",
"minWidth": 90,
"opacity": 1,
"paddingHorizontal": 19,
"paddingVertical": 8.5,
"paddingHorizontal": 20,
"paddingVertical": 9.5,
}
}
>
Expand Down Expand Up @@ -1230,8 +1230,8 @@ exports[`Button container size should reduce padding by outlineWidth in case of
"justifyContent": "center",
"minWidth": 90,
"opacity": 1,
"paddingHorizontal": 18,
"paddingVertical": 7.5,
"paddingHorizontal": 20,
"paddingVertical": 9.5,
}
}
>
Expand Down Expand Up @@ -1408,8 +1408,8 @@ exports[`Button container size should return style for large button 2`] = `
"justifyContent": "center",
"minWidth": 90,
"opacity": 1,
"paddingHorizontal": 19,
"paddingVertical": 8.5,
"paddingHorizontal": 20,
"paddingVertical": 9.5,
}
}
>
Expand Down Expand Up @@ -1591,8 +1591,8 @@ exports[`Button container size should return style for medium button 2`] = `
"justifyContent": "center",
"minWidth": 77,
"opacity": 1,
"paddingHorizontal": 15,
"paddingVertical": 5.5,
"paddingHorizontal": 16,
"paddingVertical": 6.5,
}
}
>
Expand Down Expand Up @@ -1684,7 +1684,7 @@ exports[`Button container size should return style for round button 1`] = `
"height": undefined,
"justifyContent": "center",
"opacity": 1,
"padding": 9.5,
"padding": 8.5,
"width": undefined,
}
}
Expand Down Expand Up @@ -1867,8 +1867,8 @@ exports[`Button container size should return style for small button 2`] = `
"justifyContent": "center",
"minWidth": 70,
"opacity": 1,
"paddingHorizontal": 13,
"paddingVertical": 3.5,
"paddingHorizontal": 14,
"paddingVertical": 4.5,
}
}
>
Expand Down Expand Up @@ -2055,8 +2055,8 @@ exports[`Button container size should return style for xSmall button 2`] = `
"justifyContent": "center",
"minWidth": 66,
"opacity": 1,
"paddingHorizontal": 10,
"paddingVertical": 2,
"paddingHorizontal": 11,
"paddingVertical": 3,
}
}
>
Expand Down Expand Up @@ -4191,8 +4191,8 @@ exports[`Button outline should render button with an outline 1`] = `
"justifyContent": "center",
"minWidth": 90,
"opacity": 1,
"paddingHorizontal": 19,
"paddingVertical": 8.5,
"paddingHorizontal": 20,
"paddingVertical": 9.5,
}
}
>
Expand Down Expand Up @@ -4371,8 +4371,8 @@ exports[`Button outline should render button with outline and outlineColor 1`] =
"justifyContent": "center",
"minWidth": 90,
"opacity": 1,
"paddingHorizontal": 19,
"paddingVertical": 8.5,
"paddingHorizontal": 20,
"paddingVertical": 9.5,
}
}
>
Expand Down Expand Up @@ -4461,8 +4461,8 @@ exports[`Button outline should return custom borderWidth according to outlineWid
"justifyContent": "center",
"minWidth": 90,
"opacity": 1,
"paddingHorizontal": 17,
"paddingVertical": 6.5,
"paddingHorizontal": 20,
"paddingVertical": 9.5,
}
}
>
Expand Down Expand Up @@ -4551,8 +4551,8 @@ exports[`Button outline should return disabled color for outline if button is di
"justifyContent": "center",
"minWidth": 90,
"opacity": 1,
"paddingHorizontal": 19,
"paddingVertical": 8.5,
"paddingHorizontal": 20,
"paddingVertical": 9.5,
}
}
>
Expand Down
18 changes: 3 additions & 15 deletions src/components/button/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,7 @@ class Button extends PureComponent<Props, ButtonState> {
}

getContainerSizeStyle() {
const {
outline,
avoidMinWidth,
avoidInnerPadding,
round,
size: propsSize,
outlineWidth: propsOutlineWidth
} = this.props;
const {avoidMinWidth, avoidInnerPadding, round, size: propsSize, outlineWidth: propsOutlineWidth} = this.props;
const size = propsSize || DEFAULT_SIZE;
const outlineWidth = propsOutlineWidth || 1;

Expand Down Expand Up @@ -190,14 +183,9 @@ class Button extends PureComponent<Props, ButtonState> {
minWidth: MIN_WIDTH.LARGE
};

if (outline) {
if (round) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It appears this does nothing, why not remove this completely?

_.forEach(CONTAINER_STYLE_BY_SIZE, style => {
if (round) {
style.padding -= outlineWidth; // eslint-disable-line
} else {
style.paddingVertical -= outlineWidth; // eslint-disable-line
style.paddingHorizontal -= outlineWidth; // eslint-disable-line
}
style.padding -= outlineWidth;
});
}

Expand Down