fix: display group handles RTL language direction

[jfmcquade]

PR Checklist

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

Description

Follow-up to #2589

• Now, when a display group has a background image set, the image will be flipped horizontally when the language direction is RTL (as requested in this comment).
  • We may want to expose a parameter to handle this behaviour, as some images may not be intended to be mirrored
• Additionally, fixes an undocumented RTL issue where the contents of a display group would be aligned to the left of the screen when the language was RTL.

Git Issues

Closes #

Screenshots/Videos

feature_display_group:

LTR:

RTL:

[chrismclarke]

I think perhaps this needs to be handled differently as I can imagine cases where the background image includes text that doesn't want to be flipped horizontally (e.g. logos/icons).

Could we not just use the asset language override system to include a flipped version for specific language codes?

[esmeetewinkel]

Could we not just use the asset language override system to include a flipped version for specific language codes?

I think that might not work exactly, since also an image that is e.g. aligned to the top right would need to become aligned to the top left(?)

[github-actions]

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

https://plh-teens-app1--pr2677-fix-display-group-ba-6f47u9rm.web.app

_{(expires Tue, 21 Jan 2025 14:52:57 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: e4c0bab6b08dd290fbf002fd6e07987fa4b5fce1}

[jfmcquade]

I think perhaps this needs to be handled differently as I can imagine cases where the background image includes text that doesn't want to be flipped horizontally (e.g. logos/icons).

Could we not just use the asset language override system to include a flipped version for specific language codes?

Hmm yes I did consider this but dismissed it for the reason @esmeetewinkel mentions. It's quite a tricky problem to solve, but we do have an immediate use case where it is required.

One option would be to extend this PR's implementation with an exposed property, such as background_image_flip_on_rtl (or, inversely, background_image_maintain_direction or similar), to toggle whether or not the mirroring is applied.

A messy but potentially workable option would be to author the change manually: i.e. author two sets of the display group in question, with only one displaying conditionally based upon the current app language.

[esmeetewinkel]

I don't have time to review now - but would be good to test this carefully on the plh_kids_kw deployment prior to merge because I - at some point - saw that display groups were no longer swapping as they should for RTL. Not sure this was the culprit...

[chrismclarke]

Could we not just use the asset language override system to include a flipped version for specific language codes?

I think that might not work exactly, since also an image that is e.g. aligned to the top right would need to become aligned to the top left(?)

It should still be possible to change the alignment based on language code, either via an override template or some sort of calc statement reading from @fields._app_language. Alternatively we could add a new contact field that specifically tracks whether the current language is RTL (e.g. @fields._app_language_rtl could return true/false value) which could be used with condition columns to provide multiple combinations of asset + alignment.

Or if wanting a single asset can provide a raw CSS override to flip in the same way this PR tries to hardcode - transform: scaleX(-1);

In either case I still think better to handle from the authoring, as doing it in code raises more questions about why, for example, all images aren't flipped horizontally when in rtl mode and just the background image. Better to at least be consistent

[esmeetewinkel]

It should still be possible to change the alignment based on language code, either via an override template or some sort of calc statement reading from @fields._app_language. Alternatively we could add a new contact field that specifically tracks whether the current language is RTL (e.g. @fields._app_language_rtl could return true/false value) which could be used with condition columns to provide multiple combinations of asset + alignment.

Or if wanting a single asset can provide a raw CSS override to flip in the same way this PR tries to hardcode - transform: scaleX(-1);

In either case I still think better to handle from the authoring, as doing it in code raises more questions about why, for example, all images aren't flipped horizontally when in rtl mode and just the background image. Better to at least be consistent

Okay, that's fine with me, I've been able to resolve with authoring using your first suggestion (I was worried about duplicate assets making the app download larger than needed but not a problem since it's only two assets in this case, and good to know there's a CSS workaround too). I do agree we're actually in a bit of a confusing (and inconsistent) situation already with some components flipping some images horizontally but not all. But hopefully that's mainly contained within KW specific components, and we can move towards more consistency as and when these become available to all deployments.

sheet

[esmeetewinkel]

@jfmcquade shall we close this PR favouring the workaround that @chrismclarke suggested?

[jfmcquade]

@jfmcquade shall we close this PR favouring the workaround that @chrismclarke suggested?

Yes I think so, it's true that we can't really apply this mirroring universally as it's all content-specific, so it makes sense that it should be handled at a content authoring level. If we decide further tools for doing so would be helpful, we can consider those independently of these proposed changes.

[chrismclarke]

@jfmcquade shall we close this PR favouring the workaround that @chrismclarke suggested?

Yes I think so, it's true that we can't really apply this mirroring universally as it's all content-specific, so it makes sense that it should be handled at a content authoring level. If we decide further tools for doing so would be helpful, we can consider those independently of these proposed changes.

Sounds good to me too, I've made a quick follow-up PR to provide an app language direction field for authors and give an example of flipping image horizontally on rtl language
#2715