Skip to content

Conversation

@userquin
Copy link
Contributor

releated to #317

@vercel
Copy link

vercel bot commented Jan 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
docs.npmx.dev Ready Ready Preview, Comment Jan 29, 2026 5:02pm
npmx.dev Ready Ready Preview, Comment Jan 29, 2026 5:02pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
npmx-lunaria Ignored Ignored Jan 29, 2026 5:02pm

Request Review

@userquin userquin changed the title feat(ui): AppFooter - add RTL support feat(ui): add RTL support to AppFooter Jan 29, 2026
@userquin userquin changed the title feat(ui): add RTL support to AppFooter feat: add RTL support to AppFooter Jan 29, 2026
{{ $t('footer.docs') }}
<span class="i-carbon-launch w-3 h-3" aria-hidden="true" />
<span>{{ $t('footer.docs') }}</span>
<span class="i-carbon:launch rtl-flip w-3 h-3" aria-hidden="true" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, why change the class name of the icon?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsing logic with - separator for collection and icon can be expensive; using : the collection and the icon in just one pass: i-carbon-x-y can be carbon or carbon-x collection and x-y or y icon respectivelly

@danielroe danielroe added this pull request to the merge queue Jan 29, 2026
Merged via the queue into npmx-dev:main with commit 36474fe Jan 29, 2026
13 of 15 checks passed
@userquin userquin deleted the app-footer-rtl branch January 29, 2026 17:14
<footer class="border-t border-border mt-auto">
<div class="container py-3 sm:py-8 flex flex-col gap-2 sm:gap-4 text-fg-subtle text-sm">
<div class="flex flex-col sm:flex-row items-center justify-between gap-2 sm:gap-4">
<div class="flex flex-col sm:flex-row items-center justify-start gap-2 sm:gap-4">
Copy link
Contributor

@knowler knowler Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m curious about the change away from justify-content: space-between. How does that improve RTL support? From my understanding, in flexbox layout, the main axis will flip when the direction changes. Is there something else I’m missing?

Copy link
Contributor Author

@userquin userquin Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we need to swap the left and the right inner containers => links on left for LTR and on right for RTL, I added a span between. With justify-between both container without gap (1fr between)

Copy link
Contributor

@knowler knowler Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I don’t follow. Flexbox is already writing-mode/directionality aware, so the layout contents would have already flipped before this PR (i.e. “links on left for LTR and on right for RTL” already happens). Using justify-content: start has no effect on the directionality of the items within the layout (flex-direction could have that effect, but it’s also the wrong tool for the task) and only seems to introduce the problem that requires the empty, flex-growing/shrink, ARIA hidden <span> element hack.

Usually, I wouldn’t call this out, but since it’s propagating a hack everywhere, I think it’s worth considering if it’s actually necessary. IMO the only changes relevant to RTL support in this PR is adding the rtl-flip class to the icons and changing text-left to text-start in a couple of places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to keep the html markup: A <---> B. If we want to visually change on RTL to B <---> A using justify-between, how can we do that? We need correct html markup and we need to swap containers.

How do you display an icon + button or button + icon? adding justify-start with correct html markup, on RTL we need to change the "order" (justify-start will do for us) => here we want to have a separator between them and so I added an span.

Copy link
Contributor Author

@userquin userquin Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check https://npmxdev-git-fork-userquin-app-footer-rtl-poetry.vercel.app/ adding dir="rtl" to the html in the devtools and check the links container and the paragraph below.

app footer with RTL
app footer with RTL

app footer with LTR
app footer with LTR

Copy link
Contributor

@knowler knowler Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

justify-content has no effect on the order of flex items — only the placement of items on the main axis of a flex layout (e.g. justify-content: start will have items flow from the start of the main axis). There is no need to change the order with CSS or change the markup. Like I said, flexbox (and even grid) are writing-mode/directionality aware and will change how items flow within an axis based on the directionality of the context. Internationalization has already been baked into these layouts from the start.

See: https://rtlstyling.com/posts/rtl-styling/#flexbox-layout-module

A little demo: https://knowler.dev/demos/KeLDPt7?codepen

Copy link
Contributor Author

@userquin userquin Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you provide the alternative? I want to understand how to solve it, I'm not saying your're wrong, get the original AppFooter.vue and play with it (and maybe send a PR), I'm applying the same "hack" in another components/pages (when using justify-between).

Copy link
Contributor

@knowler knowler Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR here: #340

It just reverts all of the extra spans, that hacky span, and any introduction of justify-start.

<div class="flex flex-col sm:flex-row items-center justify-start gap-2 sm:gap-4">
<p class="font-mono m-0 hidden sm:block">{{ $t('tagline') }}</p>
<div class="flex items-center gap-3 sm:gap-6">
<span aria-hidden="true" class="flex-shrink-1 flex-grow-1" />

This comment was marked as outdated.

knowler added a commit to knowler/npmx.dev that referenced this pull request Jan 29, 2026
In npmx-dev#318, `justify-content: space-between` was removed unnecessary and a
hack was introduced in its place.

Flexbox layout already is directionality/writing-mode aware and
`justify-content` has no bearing on the order of items, just their
placement within the “main” axis.

This switches back to what it was before which is simpler as it doesn’t
add extra elements, presentational elements that are hidden with ARIA.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants