feat: enable transparent or custom background for content #2429
Conversation
There was a problem hiding this comment.
One small question which you might already have looked into on this one. Also happy to be another pair of eyes here if required!
I suspect that we can check if a color is set (or if it's set back to white) to get the same data point.
Either way - this is a great one to have picked up and fixed thank you 👏
| @@ -1,6 +1,7 @@ | |||
| import { MoreInformation, parseMoreInformation } from "../shared"; | |||
|
|
|||
| export interface Content extends MoreInformation { | |||
| customBackground: boolean; | |||
There was a problem hiding this comment.
Is there any way we could derive this information from the props already passed in as opposed to adding a new prop?
Is customBackground always the same as color !== "#FFF" or whatever the default that's set is?
There was a problem hiding this comment.
Need the pizza to build to confirm this - but I think if we introduce a new required prop like this, all existing Content components will break / need to be updated which is less an than ideal! Another reason to: try to derive from existing props, or at minimum introduce this as an optional prop!
There was a problem hiding this comment.
Good shout and I appreciate the logic introduced here may not be optimal (and I hadn't considered the implications of a new prop breaking existing components).
I'll try to rework so that the padding is introduced only if the background colour is changed from the default to see if that would be a simpler solution.
There was a problem hiding this comment.
@DafyddLlyr @jessicamcinchak I've now updated this to be much simpler, doing as Daf suggested and checking whether or not the background is white rather than introducing a new option.
I am however struggling to get it working as expected, if no background colour is selected the prop is passed through empty, however trying to target this with undefined is not working.
I've also tried adding #ffffff (equivalent white output by the colour picker) as a default prop and this is still not working.
Note: I've also disabled the alpha/transparency slider in the colour picker as this appears to be non-functional, at least in Chrome and Safari.
There was a problem hiding this comment.
Thanks for the update - I'll take a look @ianjon3s - checking for a negative is always hard!
There was a problem hiding this comment.
Here's a pizza with three content components:
- Content with default background (no background colour selected)
- Content with white background selected
- Content with other custom background colour selected
https://2429.planx.pizza/testing/content-colour-test/preview
There was a problem hiding this comment.
Pizza looks great - thanks for the link ✅
|
Removed vultr server and associated DNS entries |
Co-authored-by: Dafydd Llŷr Pearson <[email protected]>
What does this PR do?
The current version of the Content component has the option for a custom background colour but by default is published with a white (transparent) background.
Horizontal and vertical padding is added to the content container by default to compensate for a background colour, meaning that the default behaviour is to push the content out of alignment with the parent container.
The PR introduces options for transparent and custom containers.
https://2429.planx.pizza/testing/content-colour-test/preview
Preview
Before (above) & after (below):