IDEMSInternational / open-app-builder

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

feat: add 3 new variants for display group: box_gray, box_primary, box_secondary #2227

Closed jfmcquade closed 4 months ago

jfmcquade commented 4 months ago

PR Checklist

Description

Adds 3 new variants for the display_group component: box_gray, box_primary and box_secondary.

Whilst there is considerable refactoring that could be done to improve the display group code, these visual variants (a grey box at least) are required for an imminent release, so the general improvements to the code are minimal.

Git Issues

Closes #2149

Screenshots/Videos

Updates to feature_display_group

Screenshot 2024-03-08 at 12 05 53 Screenshot 2024-03-08 at 12 06 41
chrismclarke commented 4 months ago

Should be allow both gray and grey? In general I think we've favoured UK spelling

haha, was exactly my question! the difference gives soo much grief from a dev perspective, I think consensus is generally for code you should be using american version (like we do for things like color and center), but given we also want to support authors likely best to have both. Similar discussion from a while back on tailwind style library: https://github.com/tailwindlabs/tailwindcss/issues/521

esmeetewinkel commented 4 months ago

(like we do for things like color and center),

Interesting you say that, the header PR that's happening in parallel uses colour (or does it allow both?)

image
chrismclarke commented 4 months ago

Interesting you say that, the header PR that's happening in parallel uses colour (or does it allow both?) image

No for that one we'll be converting from colour in the config to color in the code, so yeah might be a bit confusing for anyone that uses US-English. Might also be good to support both (ideally by converting in the parser instead of explicitly handling in the code)