shadcn-svelte icon indicating copy to clipboard operation
shadcn-svelte copied to clipboard

Simple changes to make this library support i18n use cases

Open azezsan opened this issue 1 year ago • 8 comments

Describe the feature

I have used many components from shadcn-svelte, and my project has multiple languages one of them is arabic which is RTL, the way shadcn is built is really good, I always change some small stuff to make it work even in RTL, these changes are very tiny and the ui always ends up looking the exact same so I was wondering while I don't expect shadcn/ui to have this, this isn't a whole new feature is just using different tailwind classes

change 1: replacing every space-x and space-y with gap

not only is this just more clean, it's also stable now https://caniuse.com/?search=gap

change 2: the harder pill to swallow is changing all (mr, ml, pr, pl) to (me, ms. pe, ps) 🙂

I know change 2 is not the norm but these are the only two changes that consistently make the website look the same, support RTL for i18n use cases and make this library perfect

azezsan avatar Feb 23 '24 00:02 azezsan

This is a good idea tbh.

I'll need to see how twMerge handles things before giving this the green light.

For example, if in our default classes, we have "me-2" and the consumer of the component passes "mr-4", will this override "me-2" or will it create a conflict?

huntabyte avatar Feb 23 '24 17:02 huntabyte

I have not tried that yet 🤔, because I just switch all of them and keep using "me-2" afterward but give me some time and I'll give it a try

azezsan avatar Feb 24 '24 02:02 azezsan

Thank you, @azezsan! That would be my only concern. I hope that eventually, tailwind will only support me, ms, etc... to force the hands a bit.

huntabyte avatar Feb 24 '24 02:02 huntabyte

@huntabyte Okay so, twMerge does support some of our use cases but not fully, text-start and text-end will be overridden by any text-left or text-right, it always checks which one came last

but it doesn't do it for ms or me because they could be used together

but I came up with a partial solution for this problem

import { extendTailwindMerge } from "tailwind-merge";
import { cubicOut } from "svelte/easing";
import type { TransitionConfig } from "svelte/transition";

const anyValue = () => true

const twMerge = extendTailwindMerge({ 
    extend: {
        classGroups: {
            px: [{ "ps": [anyValue]}, { "pe": [anyValue]}],
            mx: [{ "ms": [anyValue]}, { "me": [anyValue]}],
            pl: [{ "ps": [anyValue]}],
            pr: [{ "pe": [anyValue]}],
            ml: [{ "ms": [anyValue]}],
            mr: [{ "me": [anyValue]}],
        }
    }
});

export function cn(...inputs: ClassValue[]) {
    return twMerge(clsx(inputs));
}

ps in LTR simply means left, so if some LTR consumer would use this library it will behave normally

so now in our default classes, if we have "me-2" and the consumer of the component passes "mr-4", it will override "me-2"

as for the RTL consumer, it will work for him right away but he has to continue using ps pe ms etc..., but at least they start with working ui

I added mx, px to override pe, ps, ms, me if they came last since it behaves the same in left and right

azezsan avatar Feb 24 '24 11:02 azezsan

Awesome! Maybe this is something worth adding to the docs under an "Internationalization" section or similar? Showcasing how one could set this up in their projects

huntabyte avatar Feb 24 '24 19:02 huntabyte

Hmm.. I don't like that we have to add that piece of code for all consumers and having to change everything to ps pe ms etc..., it might annoy some consumers who don't care about i18n and it's deviating from shadcn-ui, maybe changing space-x, space-y to gap and text to text-start, text end, makes sense and twMerge handles it well by default

for ps pe ms etc... it could be something in the docs under "Internationalization"?

azezsan avatar Feb 24 '24 23:02 azezsan

Does gap work with elements that are just block instead of a flex or grid?

huntabyte avatar Feb 26 '24 01:02 huntabyte

no it doesn't, flex and grid only

azezsan avatar Feb 26 '24 05:02 azezsan

Sad to see this closed, would be awesome to have RTL support out of the box!

@azezsan What prompted you to close?

jakobpesch avatar Apr 20 '24 06:04 jakobpesch

Sad to see this closed, would be awesome to have RTL support out of the box!

@azezsan What prompted you to close?

there is just many minor inconveniences for users and maintainers that it's kind of annoying to deal with if the majority of users don't care and don't want to use ms. me and would prefer using ml, mr

the best suggestion I could come up with is having a option when using the cli to switch to i18n mode and it will switch everything to ms, me etc.. if the user choose but again that's not part of shadcn and will be extra work for maintainers

azezsan avatar Apr 20 '24 06:04 azezsan

Hmm, I see your point. In an ideal world people would primarily use logical css, but the majority is probably not concerned with RTL and don't bother.

I guess the ones that do need RTL support have to customise their components by hand then.

Thanks for your effort anyway :)

jakobpesch avatar Apr 20 '24 14:04 jakobpesch