[RFC] Container Background

kweku5 commented 5 years ago

Feature Request

As an author I want to apply styling to a container with other components inside of it, so I can provide a different background to a page section.

Acceptance Criteria:

Author should be able to define the colors in the component template Author should be able to define if an optional image is allowed or not in the component template Author should be able to add an image as a background, image should fill the entire container Author should be able to select the defined colors when using the component on a page When image or color is selected it will be displayed as the background in the container with all other component content displayed on top.

kweku5 commented 5 years ago

@gabrielwalt

JasonUSC commented 5 years ago

I think this is a duplicate of #415.

gabrielwalt commented 5 years ago

Hi @kweku5, thank you for your RTC; this makes sense!

@JasonUSC, you're correct that this somewhat duplicates #415. I'd suggest that we keep this RTC here just for the first initial step and immediate need of adding a background capability to the existing "Responsive Layout" container, and that we use #415 for the future proper rewrite of Responsive Layout component to provide a unified parsys with optional responsive layout and content inheritance as you described.

I'd therefore name this component just "Container", so that it is kept generic and allows for these future enhancements. For now, it would typically have a sling:resourceSuperType="wcm/foundation/components/responsivegrid", so that it wraps the Foundation Layout Container without duplicating it. To render the background image, best would probably to do like the Teaser component: to delegate the rendition to the Image component and to use the configurations of the Image component. This will however require a little CSS to display the image as a background.

The design dialog would be extended with a "Background" tab before the "Styles":

[Checkbox] "Disable images"
A note as for the Teaser component: "Image Delegate: Image rendering is delegated to the {{Component Name}} component."
[Checkbox] "Disable colors"
[Checkbox] "Use swatches only" (disabled if "Disable colors" is checked)
[Multifield of color fields] "Swatches" (disabled if "Disable color" is checked)

The edit dialog would contain just one "Background" tab:

[Image] "Image Asset" (display only if "Disable images" hasn't been checked)
[ColorInput] "Color" [1] (display only if "Disable colors" hasn't been checked)
- If "Use swatches only" has been checked, then don't display the color properties tab
- If no "Swatches" have been configured, then don't display the swatches tab

Notes:

For now, the edit dialog would look empty if both images and colors have been disabled, but later, there will be other tabs again for the responsive and inheritance capabilities.
The team has worked already on a Container POC [2]

[1] https://helpx.adobe.com/experience-manager/6-4/sites/developing/using/reference-materials/coral-ui/coralui3/Coral.ColorInput.html [2] https://github.com/adobe/aem-core-wcm-components/tree/open-friday/issues/41/content/src/content/jcr_root/apps/core/wcm/components/container (AGSIL-81)

JasonUSC commented 5 years ago

@gabrielwalt Alright, well, my immediate need in #415 was also the ability to do a background image, which is desperately needed. Our implementation of core components has been on hold for 4+ months now because this feature has not been available and we really can't go all in on our landing pages until we have this.

I do have a question about the upgrade path from this #507 container component that would use the foundation responsivegrid as its basis. When you get around to redoing this the right way, will we have an easy way to update to #415 (will #415 just be an enhancement of #507)?

gabrielwalt commented 5 years ago

@JasonUSC: Great to know that this will unblock your needs for using the core components on your landing pagesl!! To your question, it's hard to anticipate what content refactoring the rewrite of the layout container will bring with it the day we start with it (and it might be that this rewrite gets deferred further as we still have a lot of missing core component capabilities to address). However, if we change how the content persists, we'll need a migration script anyway to help migrating from the Layout Container to the new Container.

@kweku5: After discussing with one of our designers, we might also want to add a transparency option to the background images, because it seems to be a common use-case to display them half-dimmed on top of a background color.

Therefore, I'd add to the design dialog an option:

[Checkbox] "Disable image transparency slider" (disabled if "Disable images" is checked)

And a corresponding edit dialog option:

[Slider] "Image transparency" (display only if neither "Disable images" nor "Disable image transparency slider" has been checked)

JasonUSC commented 5 years ago

If there is going to be a transparency slider, the template editor ought to be able to configure the default setting through a component policy. I would hate to have authors manually guessing what the value should be and not have a consistent look.

gabrielwalt commented 5 years ago

@JasonUSC, good point. And, the colors can have transparency too.

Ao to recap the dialogs:

The Design dialog would be extended with a "Background" tab before the "Styles":

[Checkbox] "Disable images" (checking this would disable all related controls)
- [Slider] "Default transparency"
- [Checkbox] "Force default transparency"
- A note as for the Teaser component: "Image Delegate: Image rendering is delegated to the {{Component Name}} component."
[Checkbox] "Disable colors" (checking this would disable all related controls)
- [Slider] "Default transparency"
- [Checkbox] "Force default transparency"
- [Checkbox] "Use swatches only"
- [Multifield of color fields] "Swatches"

The Edit dialog would contain just one "Background" tab:

[Image] "Image Asset" (display this section only if "Disable images" hasn't been checked in Design dialog)
- [Checkbox] "Use default transparency" (checked by default; display only if "Force default transparency" has not been checked)
- [Slider] "Image transparency" (disabled if "Use default transparency" is unchecked; display only if "Force default transparency" has not been checked in Design dialog; default its value configured in the Design dialog)
[ColorInput] "Color" [1] (display this section only if "Disable colors" hasn't been checked in Design dialog)
- If "Use swatches only" has been checked, then don't display the color properties tab
- If no "Swatches" have been configured, then don't display the swatches tab
- [Checkbox] "Use default transparency" (checked by default; display only if "Force default transparency" has not been checked)
- [Slider] "Color transparency" (disabled if "Use default transparency" is unchecked; display only if "Force default transparency" has not been checked in Design dialog; default its value configured in the Design dialog)

aahlawat commented 5 years ago

@gabrielwalt @JasonUSC Shall we provide author an option to enable/disable default colors also when he has list of custom colors to add in multifield. @gabrielwalt Should we allow authors to selects both check box for background image as well as colors at template level and if author has configured both background image and color which one to display in that case.

gabrielwalt commented 5 years ago

@aahlawat, great questions!

As far as I understood the color input widget, the default colors don't make much sense, because we want that all colors can be defined as swatches by the author. Therefore, we can disable the default colors all the time, without providing a design dialog option to enable/disable the default colors.
I think that it makes sense for authors to be able to define both the background image and the color. This allows to define a transparent image that gets blended on a colored background, and it allows to define a colored fallback if images take some time to display. The color should therefore be displayed behind the image.

aahlawat commented 5 years ago

@gabrielwalt Thanks for clarifying i have implemented above features, need one small clarification about background image. How we should display background image like it should be based on content inside container or we should display full image irrespective of weather content short or long.

2) Can we change onlySwatches to display/show Properties. this is because for showing properties tab we have property showProperty and it is in negation to onlySwatches means if onlySwatches is true then showProperties will be false. If we keep display name as show/display properties then it will be in sync.

auniverseaway commented 5 years ago

FWIW, adobe.com has the following color / background needs for a container:

Mobile / Default

Background Color - Brand Colors + RGBA
Foreground Color - Brand Colors + RGBA
Background Image
- Point Of Interest X/Y Coordinates - Used for intelligent cropping
Background Video
- Play - Select
  - On enter view
  - On hover
- Loop - Boolean
- Crop video to container - Boolean
- Crop container to video - Boolean

Tablet

Inherit from previous screen size - Boolean
Background Color - Brand Colors + RGBA
Foreground Color - Brand Colors + RGBA
Background Image
- Point Of Interest X/Y Coordinates - Used for intelligent cropping
Background Video
- Play - Select
  - On enter view
  - On hover
- Loop - Boolean
- Crop video to container - Boolean
- Crop container to video - Boolean

Desktop

Inherit from previous screen size - Boolean
Background Color - Brand Colors + RGBA
Foreground Color - Brand Colors + RGBA
Background Image
- Point Of Interest X/Y Coordinates - Used for intelligent cropping
Background Video
- Play - Select
  - On enter view
  - On hover
- Loop - Boolean
- Crop video to container - Boolean
- Crop container to video - Boolean

gabrielwalt commented 5 years ago

@aahlawat, I think for the details, we'll see how it feels for now and we'll iterate over it if more is needed.

@auniverseaway, thanks for the details, we'll dive into that!

aahlawat commented 5 years ago

Feedback comments:-

1) Add color picker to design dialog instead of textfield to pick color. 2) Disable swatches tab , if no custom colors are defined 3) Re-arrange design dialog to segregate fields better. 4) Need to test container inside a container and use layout mode to test responsiveness of component

vladbailescu commented 5 years ago

Please also test responsive layout works inside the container!

richardhand commented 5 years ago

have some additional thoughts on this component:

• Foreground color. Allowing configuration of a background color without a foreground color is problematic. From a designers perspective one can't really be considered without the other. How would you be expected to influence the foreground color for all components in the container if a dark background is set and the default text color is also dark? • Edit Dialog - Swatches Only. I wouldn't allow selection of a hex/rgb value by an author in an edit dialog, only pre-defined swatches. It could lead to design inconsistencies across the site which would later be difficult to correct. • Color Disabled by Default. I would be tempted to disable background (and foreground - see above) color selection in the edit dialog by default - with only a background image configurable. The style system, in my opinion is a better way of defining these skins - with both background and foreground color considered together - and the css class applied is able to modify all nested components.

rwinterpacht commented 5 years ago

@gabrielwalt When we use responsivegrid instead of parsys, it adds extra classes that we may not want, such as aem-Grid aem-Grid--12 aem-Grid--default--12. If there's a way to provide an option to suppress that, would be great. (Something like cq:noDecoration?)

richardhand commented 5 years ago

This is available on the master branch and will be available in the upcoming 2.5.0 release.

JasonUSC commented 5 years ago

Yay!

aahlawat commented 5 years ago

Awesome Thanks Richard :)

Thanks & Regards, Ankur Ahlawat

On Thu, Jun 20, 2019 at 11:50 PM Jason Chandler notifications@github.com wrote:

Yay!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adobe/aem-core-wcm-components/issues/507?email_source=notifications&email_token=AGPMZTPBG632AMGG64FKASDP3PC67A5CNFSM4G6VNT32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYGGYYQ#issuecomment-504130658, or mute the thread https://github.com/notifications/unsubscribe-auth/AGPMZTPF6I5732DSZC6W7Y3P3PC67ANCNFSM4G6VNT3Q .

adobe / aem-core-wcm-components