-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Button: Introduce size
prop
#51842
Button: Introduce size
prop
#51842
Changes from all commits
b992cbe
5f1f4a8
82cff38
5446c80
9f703d6
ae12f01
400a02b
ccbd9da
47bc0ea
d03ba97
9ec488a
590964c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,15 +25,6 @@ type BaseButtonProps = { | |
* @default false | ||
*/ | ||
__next40pxDefaultSize?: boolean; | ||
/** | ||
* Start opting into the larger `isSmall` button size that will become the | ||
* default small size in a future version. | ||
* | ||
* Only takes effect when the `isSmall` prop is `true`. | ||
* | ||
* @default false | ||
*/ | ||
__next32pxSmallSize?: boolean; | ||
/** | ||
* The button's children. | ||
*/ | ||
|
@@ -74,8 +65,13 @@ type BaseButtonProps = { | |
* Renders a pressed button style. | ||
*/ | ||
isPressed?: boolean; | ||
// TODO: Deprecate officially (add console warning and move to DeprecatedButtonProps). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can only be done after all the usages in the Gutenberg app are migrated. |
||
/** | ||
* Decreases the size of the button. | ||
* | ||
* Deprecated in favor of the `size` prop. If both props are defined, the `size` prop will take precedence. | ||
* | ||
* @deprecated Use the `'small'` value on the `size` prop instead. | ||
*/ | ||
isSmall?: boolean; | ||
/** | ||
|
@@ -92,6 +88,18 @@ type BaseButtonProps = { | |
* If provided, renders a Tooltip component for the button. | ||
*/ | ||
showTooltip?: boolean; | ||
/** | ||
* The size of the button. | ||
* | ||
* - `'default'`: For normal text-label buttons, unless it is a toggle button. | ||
* - `'compact'`: For toggle buttons, icon buttons, and buttons when used in context of either. | ||
* - `'small'`: For icon buttons associated with more advanced or auxiliary features. | ||
* | ||
* If the deprecated `isSmall` prop is also defined, this prop will take precedence. | ||
* | ||
* @default 'default' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably add a note about the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! d03ba97 |
||
*/ | ||
size?: 'default' | 'compact' | 'small'; | ||
/** | ||
* If provided, displays the given text inside the button. If the button contains children elements, the text is displayed before them. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,6 @@ import Button from '../button'; | |
import { HStack } from '../h-stack'; | ||
import { space } from '../ui/utils/space'; | ||
import { COLORS } from '../utils'; | ||
import type { FontSizePickerProps } from './types'; | ||
|
||
export const Container = styled.fieldset` | ||
border: 0; | ||
|
@@ -44,12 +43,3 @@ export const Controls = styled.div< { | |
${ ( props ) => | ||
! props.__nextHasNoMarginBottom && `margin-bottom: ${ space( 6 ) };` } | ||
`; | ||
|
||
export const ResetButton = styled( Button )< { | ||
size: FontSizePickerProps[ 'size' ]; | ||
} >` | ||
&&& { | ||
height: ${ ( props ) => | ||
props.size === '__unstable-large' ? '40px' : '30px' }; | ||
} | ||
Comment on lines
-51
to
-54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure when it regressed, but these height overrides weren't currently working. The reset button was rendered as 24px no matter what. In this PR, I went ahead and simplified it a bit so that the reset button is 40px in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I wonder if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I definitely considered it, but a 2px difference between the text field and the button would irritate a designer more than a 6px difference would 😆 |
||
`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add a test about the interaction between
isSmall
andsize
, to make sure that they behave as expected in terms of which prop has priority?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea 9ec488a