-
Notifications
You must be signed in to change notification settings - Fork 219
Stock Indicator block: Add support for global style #5525
Changes from all commits
e9d7c30
517f80a
48709c7
01e0985
71ec323
0da8683
a194ceb
46b1927
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 |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { useBlockProps } from '@wordpress/block-editor'; | ||
import classnames from 'classnames'; | ||
|
||
type Props = { | ||
attributes: Record< string, unknown > & { | ||
className?: string; | ||
}; | ||
}; | ||
|
||
export const Save = ( { attributes }: Props ): JSX.Element => { | ||
return ( | ||
<div | ||
{ ...useBlockProps.save( { | ||
className: classnames( 'is-loading', attributes.className ), | ||
} ) } | ||
/> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { isFeaturePluginBuild } from '@woocommerce/block-settings'; | ||
|
||
export const supports = { | ||
...( isFeaturePluginBuild() && { | ||
color: { | ||
text: true, | ||
background: false, | ||
link: false, | ||
}, | ||
} ), | ||
typography: { | ||
fontSize: true, | ||
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. It makes sense to me to support all typography properties for this block instead of only font size. At least, font-weight and text-transform should be supported. 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. 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, makes sense to me. I tested the Post Categories block from Gutenberg and it offers many more typography options than what we offer in Stock indicator. I guess the same could be said about the Product Category List, Product Summary and Product Tag List. @vivialice what are your thoughts on this? Should we enable all typography properties in those blocks? |
||
}, | ||
}; |
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.
We are removing the feature that store owners actually need here. The current Supports API doesn't allow multiple colors, so I don't think it's a good idea to remove styles for stock statuses. Instead, we should accept the limitation of the Supports API. In the other words, ignoring the color, only add global style support for typography.
We can also create custom inspector settings to control colors for each state, and it will be local block style only, not global style.
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.
Before proceeding, I asked the rest of the team. Please check this discussion (p1641461102003800-slack-C02FL3X7KR6), maybe we can continue the discussion there.