Fix RTL support in metadata component

[unoduetre]

What

It was flagged that the metadata Arabic version requires structural changes. See the details in the document attached to the ticket. This change only relates to structural changes. The translations mentioned in that document have already been implemented by other tickets.

Why

Trello ticket

Visual Changes

Before

After

[unoduetre]

@matthillco I know you worked on a similar task previously. Please, let me know if you have any insights, e.g. if I need to watch out for something.

[jon-kirwan]

Thanks, @unoduetre. The changes look great and work fine. I have one suggestion. I noticed that inline-start should automatically adjust positioning based on the content direction which would mean there may be no need to manually change the float position.

@media (min-width: 40.0625em) {
    dt.gem-c-metadata__term {
        float: inline-start;
    }

    dd.gem-c-metadata__definition {
        /* float: inline-start; */
    }
}

With this setup, the metadata component content appears to display correctly for both rtl and ltr.

[unoduetre]

Yes, but float: inline-start started to be supported in e.g. Chrome 118. This version allows us to support grade C browsers as well. Do you think it makes more sense to not support grade C browsers (or they're already not supported for other reasons)?

[jon-kirwan]

Okay, I thought we could continue to use it, since it's already implemented in a few places, perhaps as a progressive enhancement. However, it'd be great to hear @matthillco's thoughts?

[matthillco]

The RTL work that I undertook did not consider grade C browsers.

Logical properties have been available in evergreen browsers since 2021 and there are diminishing returns in continuing to support older browsers with fallback code. I believe we should be bold in bringing our codebases up to date.

However, there are some dangers with this approach. RTL styles implemented with logical properties have the potential to cause a page to be unreadable for some languages in unsupported browsers. So if in doubt, we can provide the fallback physical properties as well. Of course, if we're providing the fallback then there's an argument not to use the newer logical properties at all as it's just adding unnecessary code.

This is my personal view and may not align with the majority view in light of our browser support policies. If in doubt, you might want to refer this to one of the lead front-end devs for a different perspective.

[maxgds]

The crux of it lies in this point "RTL styles implemented with logical properties have the potential to cause a page to be unreadable for some languages in unsupported browsers" - we need to ensure that we build for everyone. Our unsupported browsers may not give a good experience, but they shouldn't give an unreadable or unusable one. Without looking into the full detail of this PR I'd advise doing some testing to see the extent of the impact and making a call based on that. It may be necessary to avoid the newest methods for now.

[unoduetre]

@maxgds Below I've added screenshots for comparison between a grade C browser (older version of Chrome) and a modern browser (newer version of Chrome). This is how the page will look like if we use float: inline-start. If we keep float: right, both will look like the newer one. I think the older version is acceptable, but let me know if you don't consider it as such.

Older (if we decide to use float: inline-start)

Newer

[maxgds]

Thanks that makes it clear. I think the newer option looks acceptable. The words are all there and in the right order and properly aligned to the same side, it's just slightly undesirable wrapping which I think is fine in this instance as there's no real chance of introducing confusion between differences like
publication date: 5th May 2022
and

publication date:
5th May 2022

[unoduetre]

@maxgds Older/newer mean the browser version, but based on what you said, I understand the way it looks like in older browsers is acceptable.

@jon-kirwan I've updated the code to use inline-start.

[jon-kirwan]

Thanks @unoduetre. Just one final comment - are clear: inline-start; and padding-inline-start: 0; needed? I tested in a few browsers, and disabling them didn’t seem to have much effect. It might be we can remove both.

[unoduetre]

I think it's better to leave padding-inline-start to make sure the correct padding is set and not set through cascading. But I think clear can indeed be removed. I'll do that in a minute.

[unoduetre]

clear has been removed.

[jon-kirwan]

Thanks for the changes. All looks good to me