Skip to content
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

Slideshow + Image Banner when no container are limited to white text on image #1391

Open
nicklepine opened this issue Feb 22, 2022 · 16 comments · Fixed by #2663
Open

Slideshow + Image Banner when no container are limited to white text on image #1391

nicklepine opened this issue Feb 22, 2022 · 16 comments · Fixed by #2663
Labels
Category: Enhancement New feature or request Severity: 2 High Severity

Comments

@nicklepine
Copy link

nicklepine commented Feb 22, 2022

Describe the current behavior
When opting to remove the container on slideshow or image banner, text is always white. This is not ideal when the image a merchant wishes to use is light.

We should let merchants pick the text color, or have them pick between white or their globally set text color.

Update: May 17th, 2023
This issue was flagged by leadership twice. We will prioritize a fix

@nicklepine nicklepine added Category: Enhancement New feature or request Severity: 2 High Severity labels Feb 22, 2022
@melissaperreault
Copy link
Contributor

A few discussion points to consider

  • Remove the override of white applied to Text and Button and keep relying on the color scheme to change the elements
  • Keep the overlay opacity setting but also offer an content container opacity so we can give more flexibility and not rely on creating always dramatic banner to offer good contrast.

@kjellr
Copy link
Contributor

kjellr commented May 19, 2023

This is a tricky one because it depends so much on the overall darkness of the image. Also, in schemes with dark text colors: if we don't make the text white by default, the text will be unreadable whenever the image overlay opacity past a certain percentage. The overlay color is hardcoded to black.

As noted above, I think the simplest UX solution might be to give folks a checkbox that allows them to choose between white or the text color set in the scheme.

We might also consider changing Dawn's default homepage layout to use the content container on desktop. That won't fix the problem, but it will at least provide users with something that works less awkwardly with Color Schemes by default.

@kjellr
Copy link
Contributor

kjellr commented May 19, 2023

^ Update to my note above: it's definitely hard to arrive on a default that works for everyone, but we're already pretty far from that today. A great example is that if someone adds an Image Banner or Slideshow today, and then immediately turns off the content container, they're met with this:

19-05-f97i1-dvooz

Our own forced-white text works against us there and breaks the experience. The only way for the merchant to get around it is by uploading a dark image, or by increasing in the overlay opacity.

In light of all that, #2663 seems like a generally reasonable solution to me: It retains the white text on Dawn's homepage by default, and also allows merchants to modify colors like they would anywhere else in the theme. It's not perfect, but it still allows for more good outcomes than our current solution.

@kjellr
Copy link
Contributor

kjellr commented May 23, 2023

We found some confusing bits to #2663, so here's a more modest proposal for consideration as well. This one aims to make it more clear that the color scheme is only used when the container is displayed, by reordering settings and placing the info messaging before the color scheme setting instead of after it.

The changes are:

  1. Move the "Show container on desktop" checkbox down below the "Desktop content alignment" dropdown.
  2. Move the "Visible when container displayed." info text up, in between the "Show container on desktop" field and the color palette field, and rewrite to be more clear.
Before After
Screenshot 2023-05-23 at 11 25 10 AM Screenshot 2023-05-23 at 11 24 52 AM

cc @katycobb: do you have any thoughts on that rewrite of the "Visible when container displayed" copy there? We want to be super clear that it's in reference to the color palette control.

@melissaperreault
Copy link
Contributor

melissaperreault commented May 23, 2023

Because of the UI and space between the settings e.g. checkbox + paragraph + color_scheme, I think this could still be confusing to merchants, the space makes them disconnected. Unless we add the content as an info attribute to the checkbox setting to make them more related to each other.

We could also temporary change the label of the color_scheme setting from Color scheme to something more precise Content container color scheme, that could reinforce the link between the 2 settings?

@kimberlyoleiro
Copy link

@katycobb @kjellr what if we add the word "Container" to Color Scheme" = "Container Color Scheme"

@katycobb
Copy link

katycobb commented May 23, 2023

Hi! Agreed with @melissaperreault and @kimberlyoleiro's suggestion to change Color scheme --> Container color scheme.

I think the help text clarifying when the color scheme applies might be better placed under the color scheme itself. We could add it before the existing help text, so it'd go something like:

Color scheme is only applied when container is shown. To edit all your theme's colors…

Looking at the root issue here though - it seems like it's a problem of unpredictable text color specifically. While it's correct to say that the color scheme is only applied when the container is shown, it's still a layer of abstraction away from the problem merchants are experiencing, which is text color specifically. Thoughts on getting more granular with something like this?
Color schemes affect text colors and are only applied when container is shown.

I know in theory merchants should understand that color schemes affect text colors, but in this case it might help to extra-clarify if we believe it's a confusing experience.

Edit: another idea -

Text defaults to white unless container is shown.

@melissaperreault
Copy link
Contributor

We could add it before the existing help text

Unfortunately not possible because the color_scheme setting's info text is there by default and cannot be changed. The info attribute would show under, the way it was designed.

@kjellr
Copy link
Contributor

kjellr commented May 23, 2023

Thanks folks! So given our placement options and the copy recommendations above, does this feel better to you?

Screenshot 2023-05-23 at 1 45 35 PM

@kimberlyoleiro
Copy link

The text is great. Let's try it! Can always make changes again in the future.

@melissaperreault
Copy link
Contributor

Let's ship this! 🚀

@kmeleta
Copy link
Contributor

kmeleta commented May 24, 2023

How would we feel about updating the ordering of the image overlay opacity in image banner to better match what slideshow does? I sort of feel like those 3 settings should all be grouped?

@kjellr
Copy link
Contributor

kjellr commented May 24, 2023

I don't mind moving the overlay opacity control. It makes sense to keep it consistent.

Regarding the idea here though, we didn't consider just adding a header there. I think this is probably a simpler solution, and avoids us having to add yet another line of explanatory text to the theme:

Screenshot 2023-05-24 at 10 28 56 AM

It shows that the color scheme is clearly tied to the container. It does not clarify about the white text behavior, but I think that's ok in this case.

@kmeleta
Copy link
Contributor

kmeleta commented May 24, 2023

It makes sense to keep it consistent.

That, but also the white text is meant to play off the black overlay if I'm not mistaken. So it would make sense to keep that near any note or setting related to the white text. Though if we remove that info text, it still doesn't hurt jusr probably not as impactful.

Regarding #1391 (comment), we didn't consider just adding a header there. I think this is probably a simpler solution, and avoids us having to add yet another line of explanatory text to the theme:

We should add a heading for sure, but I personally wouldn't mind both if this is a real pain point for people. There's not that much helper text in these particular sections right now.

@kimberlyoleiro
Copy link

My gut says that 2 things are confusing to merchants: 1. the container checkbox setting is just getting lost in the hierarchy 2. no one knows what a container is.

To Ken's point, I think using a mix of header and helper text can solve both problems.

What do y'all think of this? @kmeleta @kjellr @katycobb @melissaperreault

Header: Content Container (Provides context about what the container actually does)
Helper text: A container applies a color scheme to your content. Without a container, the text is white for easier reading.
Checkbox: Show container on desktop
Colour Scheme Picker: Container color scheme

@katycobb
Copy link

Most aligned with @kjellr's suggestion here:

image

  • Agree we could have the header say Content container
  • If we want to include help text to clarify the white text behavior (which I think was the main concern?) I'd suggest using the orig Text defaults to white unless container is shown. under the checkbox, which is the decision point where you'd want that info. I think this provides just enough info to help, without being too overwhelming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Enhancement New feature or request Severity: 2 High Severity
Projects
None yet
6 participants