-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Components: Allow to apply isPressed prop to Button and SVG components to indicate the state of the button #17748
Changes from all commits
442afdd
128486b
ce164e1
c03f9a6
cdc9788
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 |
---|---|---|
@@ -1,3 +1,3 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`Basic rendering should display with required props 1`] = `"<span><div tabindex=\\"-1\\"><div><div><div class=\\"components-popover block-editor-link-control\\"><div class=\\"components-popover__content\\" tabindex=\\"-1\\"><div class=\\"block-editor-link-control__popover-inner\\"><div class=\\"block-editor-link-control__search\\"><form><div class=\\"components-base-control editor-url-input block-editor-url-input block-editor-link-control__search-input\\"><div class=\\"components-base-control__field\\"><input type=\\"text\\" aria-label=\\"URL\\" required=\\"\\" placeholder=\\"Search or type url\\" role=\\"combobox\\" aria-expanded=\\"false\\" aria-autocomplete=\\"list\\" aria-owns=\\"block-editor-url-input-suggestions-0\\" value=\\"\\"></div></div><button type=\\"reset\\" disabled=\\"\\" aria-label=\\"Reset\\" class=\\"components-button components-icon-button block-editor-link-control__search-reset\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-no-alt\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></form></div></div></div></div></div></div></div></span>"`; | ||
exports[`Basic rendering should display with required props 1`] = `"<span><div tabindex=\\"-1\\"><div><div><div class=\\"components-popover block-editor-link-control\\"><div class=\\"components-popover__content\\" tabindex=\\"-1\\"><div class=\\"block-editor-link-control__popover-inner\\"><div class=\\"block-editor-link-control__search\\"><form><div class=\\"components-base-control editor-url-input block-editor-url-input block-editor-link-control__search-input\\"><div class=\\"components-base-control__field\\"><input type=\\"text\\" aria-label=\\"URL\\" required=\\"\\" placeholder=\\"Search or type url\\" role=\\"combobox\\" aria-expanded=\\"false\\" aria-autocomplete=\\"list\\" aria-owns=\\"block-editor-url-input-suggestions-0\\" value=\\"\\"></div></div><button type=\\"reset\\" disabled=\\"\\" aria-label=\\"Reset\\" class=\\"components-button components-icon-button block-editor-link-control__search-reset\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\" class=\\"dashicon dashicons-no-alt\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></form></div></div></div></div></div></div></div></span>"`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`Basic rendering should render with required props 1`] = `"<fieldset class=\\"block-editor-responsive-block-control\\"><legend class=\\"block-editor-responsive-block-control__title\\">Padding</legend><div class=\\"block-editor-responsive-block-control__inner\\"><div class=\\"components-base-control components-toggle-control block-editor-responsive-block-control__toggle\\"><div class=\\"components-base-control__field\\"><span class=\\"components-form-toggle is-checked\\"><input class=\\"components-form-toggle__input\\" id=\\"inspector-toggle-control-0\\" type=\\"checkbox\\" aria-describedby=\\"inspector-toggle-control-0__help\\" checked=\\"\\"><span class=\\"components-form-toggle__track\\"></span><span class=\\"components-form-toggle__thumb\\"></span><svg class=\\"components-form-toggle__on\\" width=\\"2\\" height=\\"6\\" xmlns=\\"http://www.w3.org/2000/svg\\" viewBox=\\"0 0 2 6\\" role=\\"img\\" aria-hidden=\\"true\\" focusable=\\"false\\"><path d=\\"M0 0h2v6H0z\\"></path></svg></span><label for=\\"inspector-toggle-control-0\\" class=\\"components-toggle-control__label\\">Use the same padding on all screensizes.</label></div><p id=\\"inspector-toggle-control-0__help\\" class=\\"components-base-control__help\\">Toggle between using the same value for all screen sizes or using a unique value per screen size.</p></div><div class=\\"block-editor-responsive-block-control__group block-editor-responsive-block-control__group--default\\"><div class=\\"components-base-control\\"><div class=\\"components-base-control__field\\"><label class=\\"components-base-control__label\\" for=\\"inspector-select-control-0\\"><span aria-describedby=\\"rbc-desc-0\\">All</span><span class=\\"screen-reader-text\\" id=\\"rbc-desc-0\\">Controls the padding property for All viewports.</span></label><select id=\\"inspector-select-control-0\\" class=\\"components-select-control__input\\"><option value=\\"\\">Please select</option><option value=\\"small\\">Small</option><option value=\\"medium\\">Medium</option><option value=\\"large\\">Large</option></select></div></div><p id=\\"all\\">All is used here for testing purposes to ensure we have access to details about the device.</p></div></div></fieldset>"`; | ||
exports[`Basic rendering should render with required props 1`] = `"<fieldset class=\\"block-editor-responsive-block-control\\"><legend class=\\"block-editor-responsive-block-control__title\\">Padding</legend><div class=\\"block-editor-responsive-block-control__inner\\"><div class=\\"components-base-control components-toggle-control block-editor-responsive-block-control__toggle\\"><div class=\\"components-base-control__field\\"><span class=\\"components-form-toggle is-checked\\"><input class=\\"components-form-toggle__input\\" id=\\"inspector-toggle-control-0\\" type=\\"checkbox\\" aria-describedby=\\"inspector-toggle-control-0__help\\" checked=\\"\\"><span class=\\"components-form-toggle__track\\"></span><span class=\\"components-form-toggle__thumb\\"></span><svg width=\\"2\\" height=\\"6\\" xmlns=\\"http://www.w3.org/2000/svg\\" viewBox=\\"0 0 2 6\\" class=\\"components-form-toggle__on\\" role=\\"img\\" aria-hidden=\\"true\\" focusable=\\"false\\"><path d=\\"M0 0h2v6H0z\\"></path></svg></span><label for=\\"inspector-toggle-control-0\\" class=\\"components-toggle-control__label\\">Use the same padding on all screensizes.</label></div><p id=\\"inspector-toggle-control-0__help\\" class=\\"components-base-control__help\\">Toggle between using the same value for all screen sizes or using a unique value per screen size.</p></div><div class=\\"block-editor-responsive-block-control__group block-editor-responsive-block-control__group--default\\"><div class=\\"components-base-control\\"><div class=\\"components-base-control__field\\"><label class=\\"components-base-control__label\\" for=\\"inspector-select-control-0\\"><span aria-describedby=\\"rbc-desc-0\\">All</span><span class=\\"screen-reader-text\\" id=\\"rbc-desc-0\\">Controls the padding property for All viewports.</span></label><select id=\\"inspector-select-control-0\\" class=\\"components-select-control__input\\"><option value=\\"\\">Please select</option><option value=\\"small\\">Small</option><option value=\\"medium\\">Medium</option><option value=\\"large\\">Large</option></select></div></div><p id=\\"all\\">All is used here for testing purposes to ensure we have access to details about the device.</p></div></div></fieldset>"`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ export function Button( props, ref ) { | |
isLarge, | ||
isSmall, | ||
isTertiary, | ||
isToggled, | ||
isPressed, | ||
isBusy, | ||
isDefault, | ||
isLink, | ||
|
@@ -33,14 +33,16 @@ export function Button( props, ref ) { | |
'is-large': isLarge, | ||
'is-small': isSmall, | ||
'is-tertiary': isTertiary, | ||
'is-toggled': isToggled, | ||
'is-pressed': isPressed, | ||
'is-busy': isBusy, | ||
'is-link': isLink, | ||
'is-destructive': isDestructive, | ||
} ); | ||
|
||
const tag = href !== undefined && ! disabled ? 'a' : 'button'; | ||
const tagProps = tag === 'a' ? { href, target } : { type: 'button', disabled }; | ||
const tagProps = tag === 'a' ? | ||
{ href, target } : | ||
{ type: 'button', disabled, 'aria-pressed': isPressed }; | ||
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. Do we have default styles for the isPressed variation of the Button 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. As commented: no, we don’t, we should do it as the follow up. 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. Looks like 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. @aduth - do you have recommendations here? I don't think we made it the rule. I'm happy to open a follow-up to update if necessary. I also want to have 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 used to prefer the un-prefixed My recommendation would be to prefer to name new boolean props using the |
||
|
||
return createElement( tag, { | ||
...tagProps, | ||
|
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.
I didn't keep the old name because it only applied the
is-toggled
class name which was never styled. I can bring it back, but I don't feel like it's a good idea to keep it. It's only confusing.