IDEMSInternational / open-app-builder

PLH App Frontend
GNU General Public License v3.0
5 stars 24 forks source link

feat: allow style_list to apply directly to text in text component #2291

Closed jfmcquade closed 2 months ago

jfmcquade commented 2 months ago

PR Checklist

Description

There has been an author request to change the colour of text that appears in a text component.

A thorough solution would involve expanding our WIP variant/style system (#1971), e.g. by adding a general style that can be applied via a parameter to any component, such as style: color-secondary-500. Currently, this is not a priority and we don't have the capacity to implement such a system properly at this time. Another option would be to add a new property on the component, e.g. colour(/color), that could take values such as primary, secondary, white, black (encouraged) and potentially also red and #32a852 etc. (discouraged/undocumented). But implementing that would be in conflict with our plan for a more general variant/style system.

This PR implements an alternative solution: the rules specified in the style_list are now applied such that they should be reflected in the appearance of the actual text itself (as opposed to the enclosing row container). This feature, or something like it, is included in our plans for the variant/style system, with this functionality rolled out to all components in a custom way.

Git Issues

Closes #

Screenshots/Videos

Updated comp_text component:

Screenshot 2024-04-11 at 18 25 11 Screenshot 2024-04-11 at 18 24 32

debug_text_style_list component demonstrating that applying a value for the flex property in the style list functions as expected, which is a pattern used in some places. (Dev note: the style is applied both to the row container element as before, and additionally to the <div> that contains the text html. The latter has no effect since the div is not inside a flexbox)

Screenshot 2024-04-11 at 18 25 47 Screenshot 2024-04-11 at 18 09 49
chrismclarke commented 2 months ago

I think exposing custom css overrides at component level makes sense, much better than adding arbitrary custom styles. For the text component this is pretty intuitive as there's only a single component so I think the solution makes sense (ignoring the slightly messy implementation of the text component more generally).

When thinking about other row types which have multiple inner components we will need a better way to signify what part to style, e.g. text_style, container_style etc. I'm assuming all this is captured in one of the working documents somewhere...