Kuwait Theme: Component styling Updates

[FaithDaka]

PR Checklist

• PR title descriptive (can be used in release notes)

Description

• plh_kids_kw theme: Changes to base components styling (text, input box, speech-bubbles, select-text, accordion)
• Corresponding component sheets (accordion and text-bubble) updated to include more example styles/variations.

Author Notes

Text Bubble Variants

• 2 new variants added to the text bubble variant parameter_list: accent-1 and accent-2.
  For the Kuwait theme the accent-1 variant corresponds to the orange text bubble while the accent-2 variant corresponds to the yellow text bubble, as used in the comics. Refer to screenshot in column 1 for visuals.
• The primary variant is the main variant used by the facilitator within the module articles.
• The secondary variant is used on the feelings check-in page and has a gold outline.
• The gray variant is used in the quiz response when a user selects an answer to the question asked. It has a gray outline.

Text Bubble Speaker Name

• Support has been added for a speaker name within the text bubble. Parameter: speaker_name Type: string
  Example - speaker_name: Dalal;

Git Issues

Closes #

Screenshots

Component Name	Default	Kuwait
text-bubble
text-area
text
text-box
select-text
accordion

[FaithDaka]

@esmeetewinkel After these styling changes are reviewed and merged, you should have more variants for the text-bubble component and styling fix for the accordion.

[jfmcquade]

Thanks, @FaithDaka, it's great to see the visual changes, and nice to have the screenshots clearly displayed in the PR description for comparison.

I've left few comments inline, we can discuss in our meeting shortly

[jfmcquade]

I think rather than "orange" and "yellow", these should be given general names so that the specific colours can be different for a different theme.

I would suggest maybe accent-1 and accent-2? We don't necessarily need to actually expose "accent" colours to the theme generation yet, but naming these variants in a way that is compatible with exposing the colours to the themes should prevent issues in the future.

[FaithDaka]

Thanks for highlighting this @jfmcquade. The variant names have been updated within the code and component sheet

[chrismclarke]

@jfmcquade - I'm not sure if we want to be introducing an accent variable when using primary and secondary (we previously agreed adding tertiary and quaternary was super confusing and removed from legacy code, so I don't see how accent-1 and accent-2 is much different). If however the accent is specific to the component and not the theme, then again I don't see the benefit in calling it something other than what it is, i.e. the orange or yellow variant.

If we're at the point of just wanting a fixed, hardcoded colour would it not make more sense to expose a parameter_list variable to manually override? Although I guess this brings up a follow-up question of whether this should be a CSS colour or another named colour from a list like https://tailwindcss.com/docs/customizing-colors

[jfmcquade]

@chrismclarke Yeh I take your point, it may not make sense to introduce another theme variable here. I think I was anticipating that we'll soon have another deployment making use of this component in various colours (@esmeetewinkel I believe there's one such deployment on the horizon?), and in that case we're likely to want the colours to be different to the ones used here. According to our incomplete style/variant system (see #1971), I think it's implicit that a style might specify a specific property value (like a colour), whereas a variant should have a more general name that can be interpreted differently by different themes. In that case, the current colour variants might make more sense as "styles" in that system, although we haven't implemented styles in this way yet – also, the fact that the variant changes both the background and border colour does mean it might be suited to have those style changes "wrapped up" in a named variant.

@FaithDaka has highlighted that from a design perspective, we are going to want to offer more than two theme level colour variables at some point (the plh_kids_kw designs uses a palette in line with the material design colour system, with at least 3 or 4 colours used in various shades). The yellow colour used in the text-bubble component variant is already being used in a different shade in the select-text component styling changes included in this PR. Currently this is hardcoded, and I was thinking that the process of adapting our existing theme system would occur at the same time as incorporating (a subset of) the plh_kids_kw style changes into our other themes, i.e. after the plh_kids_kw deployment has been released.

In any case, as we're likely not actually implementing the new theme-level colour variables at this point, I'm happy to revert these to explicit colour names. We'll likely need to give it some more thought about how to incorporate additional theme colours beyond the current primary/secondary. But I think this is something we will need to support at some point, and it could be good to get your perspective then, @FaithDaka

[chrismclarke]

Hmm, ok in that case I might backtrack a little here. So what I understand is that we now have multiple components that are likely to want to use 3+ colour variants all within the same app (e.g. multiple speaker text bubbles).

I think what makes the most sense here is to expose a parameter_list option alongside CSS variable. If we have a in the component styles

--text-bubble-color-1: var(--color-primary);
--text-bubble-color-2: var(--color-secondary);
--text-bubble-color-3: var(--color-primary);
--text-bubble-color-4: var(--color-secondary)

And then any deployment can simply provide overrides as needed in the theme styles. E.g.

--text-bubble-color-3: orange;
--text-bubble-color-4: yellow;

A little extra CSS magic would be needed to handle either making the border a darker shade or background a lighter shade depending on how we want to interpret the provided colour (although still think easier that than exposing two variables for each colour, e.g. --text-bubble-color-bg-1 and --text-bubble-color-border-1)

Using the parameter list we can expose (if not already there) something like speaker_number: 1 used to handle position and colour of multiple speakers.

What do you think @jfmcquade ?

[chrismclarke]

(oh, and will need to change the component style demo slightly so that the component style inherits from the global style and not override as my example would do)

[esmeetewinkel]

Using the parameter list we can expose (if not already there) something like speaker_number: 1 used to handle position and colour of multiple speakers.

FYI I'm handling the position, colour, icon and name of the speaker in a combination of a data list and template:

data list of characters

template authoring the speech bubble

[jfmcquade]

I think what makes the most sense here is to expose a parameter_list option alongside CSS variable. If we have a in the component styles

I think this is a clever solution. However after trying to implement it I'm not sure it quite works: if we replace the position and variant parameters with a single "speaker_number" parameter, then we lose the ability to customise the variant and position independently (for example authoring a primary coloured bubble on the right hand side, or a grey bubble anywhere), but if we keep the variant and position properties but add an additional speaker_number property, then this latter property is redundant apart from being a more efficient way to author combinations of variant and position.

As I see it, the problem we're trying to solve is to expose an authoring option to set the colour without naming a specific colour (e.g. "orange") or using a colour name that suggests a theme colour that doesn't actually exist (e.g. "accent-1").

I've implemented a sort of compromise with 71bd2d3: the third and fourth colour variants are still authored using an explicitly named variant, but these variants are named "speaker-3" and "speaker-4", to avoid naming a colour or theme-colour. The colours associated with those variants are exposed as theme-level variables, with fallbacks as the theme primary and secondary colours, as you suggested.

A little extra CSS magic would be needed to handle either making the border a darker shade or background a lighter shade depending on how we want to interpret the provided colour (although still think easier that than exposing two variables for each colour, e.g. --text-bubble-color-bg-1 and --text-bubble-color-border-1)

I've opted to use two explicit variables for each in order to keep it fully customisable (and, as we're limited to runtime CSS functions to manipulate the values of CSS var(--...)s, I wasn't too pleased with the available options for darkening/lightening the colour dynamically)

Let me know what you think of the changes in 71bd2d3, @chrismclarke and @FaithDaka

[jfmcquade]

Good to have this comment in line with our documentation convention. It would also be good to explicitly list the new parameter in the PR description, that way @esmeetewinkel can functionally test the new functionality without needing to engage with the code, plus the PR can act as a form of documentation for future reference.

The same goes for the new variants: it would be good to explicitly lit them in the PR description

[FaithDaka]

The PR description has been updated to list the new parameters @jfmcquade

[jfmcquade]

In order to preserve consistency with non-Kuwait themes, we might need to make this element hidden on every other theme and only display it when the plh_kids_kw is active. Otherwise, we should style it so that it looks good on the other themes too. It would also be good to add an example to the comp_text_bubble template so that we can see how it displays

[github-actions]

Visit the preview URL for this PR (updated for commit 71bd2d3):

https://plh-teens-app1--pr2521-style-update-comp-st-h60inx5m.web.app

_{(expires Mon, 02 Dec 2024 12:06:39 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: e4c0bab6b08dd290fbf002fd6e07987fa4b5fce1}