[AMP Stories] Add overlay settings to Page

[miina]

See #2013.

Adds Overlay settings to page which includes two color settings and opacity.
If one color is selected then the background is filled with the color, otherwise gradient is used.

[miina]

I'm wondering if it would be better to add a radio button for using Gradient. This way it would also be possible to use gradient with transparent.

At this moment if the "main" color is not selected and the second is then the gradient is created from transparent to the second color. However, the opposite case is not possible.

So perhaps it's better to have a radio to specifically set if the user wants to use gradient or not and then display either one or two color settings accordingly. Or maybe you have better alternatives in mind? Thoughts?

cc @swissspidy @westonruter

[swissspidy]

Suggested change

	title={ __( 'Overlay Settings', 'amp' ) }
	title={ __( 'Background Color', 'amp' ) }

If one just wants to use a solid background color without any gradient, I think this wording makes more sense.

Thanks to the opacity control further down, one will quickly discover that this can be used as an overlay for the background image.

[miina]

Hmm, I was thinking that Overlay is a word that is valid for both cases since Overlay doesn't necessarily have to be an overlay for an image -- it can be overlay for the Page block without the image, too. Also, technically it'll be a new Fill layer (aka overlay) and not use the background color.

Since the user will not necessarily open the Panel to see what's inside then maybe Background Color could be deceiving, it would be great if the title would be self-explanatory without having to know what's inside. Maybe there's another option instead?

[swissspidy]

Background Settings maybe? Then it could be combined with the background media panel.

Technically it's a fill layer, but for the user it's basically a background color setting with some optional opacity.

For me, "background" is easier to grasp than "overlay", and also easier to localize.

[miina]

Background Settings makes sense 👍

[amedina]

Since from the user perspective the net effect of this setting is providing a background color/fill, I would consider Background Color over `Settings. We want to make the language and UI as straightforward and intuitive as possible.

[swissspidy]

What if we want to support three or more colors in the future?

For that, I like Weston's suggestion of making this a repeatable field.

By default you'll only see the one background color option with the opacity control.

Below the color option could be an "Add Color" button to add a second color. In the future we'd allow adding a third, fourth, etc. color as well.

To make this flexible and future-proof enough, there could only be a single background attribute that is a JSON-encoded object that looks more like this:

{
  "background": {
    "opacity": 0.5,
    "colors": [
      {
        "color": "#ffffff"
      },
      {
        "color": "#000000"
      }
    ]
  }
}

For a regular solid background:

{
  "background": {
    "opacity": 1,
    "colors": [
      {
        "color": "#ababab"
      }
    ]
  }
}

Eventually, we'd need to look at adding a gradient picker à la http://www.colorzilla.com/gradient-editor/, for which we'd just need to extend it like this:

{
  "background": {
    "opacity": 0.5,
    "direction": 57,
    "colors": [
      {
        "color": "#ffffff",
        "step": 0
      },
      {
        "color": "#777777",
        "step": 0.5
      },
      {
        "color": "#000000",
        "step": 1
      }
    ]
  }
}

This way we don't have rename block attributes in the future and things like that.

Hope that makes sense.

Also, FWIW, there are a few React gradient picker components out there from which we could draw inspiration from.

[miina]

This way we don't have rename block attributes in the future and things like that.

Yep, it makes sense, thought that adding a repeating field just for two colors would be too much. However, it's true that we might need it in the future and reworking this wouldn't be the best option later.

Will add the repeating field for the two colors for now, probably we can add other improvements later (considering the timeframe).

[swissspidy]

Yep, it makes sense, thought that adding a repeating field just for two colors would be too much.

For two colors in the beginning we could label the button "Add Gradient"

[miina]

We'll also need to add remove button as well then.

[swissspidy]

I fixed some issues around edge cases, e.g. clicking on "Add Gradient" crashed the block when the first color hasn't been set yet. Now it seems to work fine.

[swissspidy]

Test_AMP_Validation_Manager::test_add_block_source_comments seems to be failing on Travis with the latest_posts data set. Opened an issue for that as it's unrelated to this PR: #2053