[Feature]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl #719

lord007tn · 2024-02-21T15:35:49Z

Reproduction

https://codesandbox.io/p/devbox/shadcn-vue-selectdemo-forked-gxptxq

Describe the bug

I set global dir for html to be dir='rtl'
Select, calendar, dropdown seems to have a default dir='ltr' so everytime i should force them to be dir='auto' so they can have parent HTML dir
it seems like auto is not a valid value in radix components
so we need to support it in shadcn components as default value

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (32) x64 AMD Ryzen 9 7950X 16-Core Processor
    Memory: 6.19 GB / 31.16 GB
  Binaries:
    Node: 18.18.2 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 9.8.1 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.14.3 - ~\AppData\Local\pnpm\pnpm.CMD
  Browsers:
    Edge: Chromium (121.0.2277.128)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @vueuse/core: ^10.7.2 => 10.7.2
    radix-vue: ^1.4.7 => 1.4.7
    vue: ^3.4.19 => 3.4.19

Contributes

I am willing to submit a PR to fix this issue
I am willing to submit a PR with failing tests

The text was updated successfully, but these errors were encountered:

zernonia · 2024-02-22T04:25:56Z

You need to wrap your app around with <ConfigProvider>. https://www.radix-vue.com/utilities/config-provider.html#anatomy

We can add some guide for user concern about RTL config in the docs.
Maybe https://www.shadcn-vue.com/docs/guide/rtl.html

lord007tn · 2024-02-22T22:58:48Z

You need to wrap your app around with <ConfigProvider>. radix-vue.com/utilities/config-provider.html#anatomy

We can add some guide for user concern about RTL config in the docs. Maybe shadcn-vue.com/docs/guide/rtl.html

Seems fine but that should be reactive
so multi-direction website can benefit from both and component change according to global direction

correct me if am wrong but i think that the <ConfigProvider> not valid in this case

zernonia · 2024-02-23T00:39:52Z

@lord007tn what do you mean "should be reactive". It is reactive
https://stackblitz.com/edit/3jaxbt?file=src%2FApp.vue

Can you explain more what is "multi-direction website", and what is the expected outcome that you are looking for?

Saeid-Za · 2024-02-27T08:54:59Z

From my understanding, I think that @lord007tn is suggesting that CSS should handle directionality based on the root element, rather than using JavaScript (ConfigProvider). I have not read the source code for ConfigProvider to determine if it can be removed. If this approach aligns with the project's goals, I would be happy to assist in implementing a CSS-only solution.

With this approach, by only changing the dir attribute on html element, we could change the directionality of web app, add some internationalization flavor and vola, we got multi-language (with different directionality) website with minimum effort (Still blocked by unovue/shadcn-vue#357)

sadeghbarati · 2024-02-27T09:30:31Z

@Saeid-Za

ConfigProvider is more like a Context for the entire App to change behavior of FloatingUi functionality to respect the direction.

I like the idea instead of ConfigProvider we just set direction on <html> tag 💯,
But html is outside of Vue's scope, handle reactivity outside of Vue is hard do you have any idea how to handle that?

My idea is to use useMutationObserver

Saeid-Za · 2024-02-27T09:39:58Z

@sadeghbarati
Thanks for response!
Oh, I see, is there no further use-case for ConfigProvider then?
Yes I would also suggest using a mutation observer on html element. but I think this is already supported by awesome folks at VueUse

useTextDirection composable
source code for better context

these are the defaults:

export interface UseTextDirectionOptions extends ConfigurableDocument {
  /**
   * CSS Selector for the target element applying to
   *
   * @default 'html'
   */
  selector?: string
  /**
   * Observe `document.querySelector(selector)` changes using MutationObserve
   *
   * @default false
   */
  observe?: boolean
  /**
   * Initial value
   *
   * @default 'ltr'
   */
  initialValue?: UseTextDirectionValue
}

We could use {observe: true} to achieve what is desired.

Update

There is this issue of first render on SSR and what defaultValue should be given to this composable, which IMO should be provided by the user, but by doing so, this approach would require user to "provide" the initial direction to radix (which is similar and constrained to limitations of ConfigProvider that was discussed before)
Any thoughts?

sadeghbarati · 2024-03-01T18:47:54Z

@zernonia

zernonia · 2024-03-02T07:52:11Z

I believe we can have this feature in radix-vue components 😁

Saeid-Za · 2024-03-03T16:16:05Z

Very well then!
Here is my proposal for implementation design of a ConfigProvider replacement/enhancement. There are several ways to implement such feature:

The Challenge here is how to provide first direction state for SSR compatibility, the rest of the implementation is described before in #719 (comment) and #719 (comment).

Using Wrapper Component (Current Implementation)

<script setup lang="ts">
import { ConfigProvider } from 'radix-vue'
</script>

<template>
  <ConfigProvider initialDir="ltr">
    <slot />
  </ConfigProvider>
</template>

We would only need to rename dir field in ConfigProviderContextValue to initialDir because we are changing the meaning of the variable in this implementation.

Using Vue Plugin

By using a plugin we could provide a global configuration, from vue docs, one of the use cases for vue plugins are:

Make a resource injectable throughout the app by calling app.provide().

user would create such plugin and this value would be used as initial value on reactive direction implemented on radix codebase.

import { createRadixConfig } from "radix-vue"
import { createApp } from 'vue'
import App from './App.vue'
import './style.css'

const radixConfig = createRadixConfig({
  initialDir: 'ltr',
})

createApp(App)
.use(radixConfig)
.mount('#app')

I'd personally recommend going this direction as it has no overhead of a render-less component and aligns better with vue itself, IMO using Provider components are better left in react, as far a I know bigger parts of vue ecosystem does not follow Provider component pattern.
Although it does have a bit lower DX compared to previous approach, it could be encapsulated to a module for nuxt but for the previous approach there is no such luxury. there is a bit trade-off here.

Let me know your thoughts and if this sound OK to you, I'd be happy to helping execute this idea.

zernonia · 2024-03-04T14:49:03Z

Thanks for awesome suggestion @Saeid-Za !

The reason with ConfigProvider components is so that the config doesn't ties to only one setup (namely main.ts).
Every page or component can have independant config, which is what Vue plugin does not offer.

Even though we can export a custom provide function to provide and inject similar config, but the idea of "wrapping" with components is more appealing to me.

What do you think?

Saeid-Za · 2024-03-05T11:21:15Z

@zernonia
Thanks for the explanation !
I haven't thought of such a use case, I thought that this is a global level config level provider, you are absolutely right.

Considering the the fact that initialDir is indeed global level due to it's scope being related to SSR, I prefer it being a plugin level provide.

Nevertheless, ConfigProvider could not be replaced which i thought it could, here's an idea, why not support both approaches?

With this new approach, the flexibility of current implementation is preserved and any plan for adding nuxt module level configuration would be open.

zernonia · 2024-03-06T02:26:46Z

We can do that @Saeid-Za .

However, I also thought about another possible solution, we can utilize useTextDirection and ConfigProvider together.
https://stackblitz.com/edit/7kcx91?file=src%2FApp.vue

This way it offers much more flexibility to the user, also support Vite/Nuxt.

Saeid-Za · 2024-03-06T03:26:57Z

Since users should always provide something to the provider for direction to work, it seems that there is no way around it.
I like the simplicity of your approach and overall i think it is the best approach.

There is no code changes required right?
Only new documentation regarding Directionality of the App

zernonia · 2024-03-06T05:32:34Z

Yup. I think so. We can add a guide to the doc about "internationalization (RTL)" 👍🏻

Would really appreciate if you can help with this 😁 With your knowledge with RTL I believe it would be easy

Saeid-Za · 2024-03-06T07:12:48Z

Sure !
I'll add documentation for i18n integrations and multi direction support.

* docs: adding docs, resolves #719 * docs: minor change --------- Co-authored-by: zernonia <[email protected]>

lord007tn added the bug Something isn't working label Feb 21, 2024

lord007tn mentioned this issue Feb 21, 2024

[Feature]: Support RTL in shadcn components unovue/shadcn-vue#357

Open

5 tasks

sadeghbarati added documentation Improvements or additions to documentation and removed bug Something isn't working labels Feb 22, 2024

hrynevychroman assigned hrynevychroman and lord007tn and unassigned hrynevychroman Feb 22, 2024

zernonia transferred this issue from unovue/shadcn-vue Mar 2, 2024

zernonia changed the title ~~[Bug]: Shadcn Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl~~ [Feat]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl Mar 2, 2024

zernonia changed the title ~~[Feat]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl~~ [Feature]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl Mar 2, 2024

zernonia linked a pull request Mar 11, 2024 that will close this issue

docs: i18n docs, resolves #719 #740

Merged

zernonia closed this as completed in #740 Mar 11, 2024

zernonia added a commit that referenced this issue Mar 11, 2024

docs: i18n (rtl) (#740)

8fb66f9

* docs: adding docs, resolves #719 * docs: minor change --------- Co-authored-by: zernonia <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl #719

[Feature]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl #719

lord007tn commented Feb 21, 2024 •

edited

Loading

zernonia commented Feb 22, 2024

lord007tn commented Feb 22, 2024

zernonia commented Feb 23, 2024

Saeid-Za commented Feb 27, 2024 •

edited

Loading

sadeghbarati commented Feb 27, 2024

Saeid-Za commented Feb 27, 2024 •

edited

Loading

sadeghbarati commented Mar 1, 2024

zernonia commented Mar 2, 2024

Saeid-Za commented Mar 3, 2024 •

edited

Loading

zernonia commented Mar 4, 2024

Saeid-Za commented Mar 5, 2024

zernonia commented Mar 6, 2024

Saeid-Za commented Mar 6, 2024

zernonia commented Mar 6, 2024 •

edited

Loading

Saeid-Za commented Mar 6, 2024

[Feature]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl #719

[Feature]: Select, Calendar, DropDown by default set dir to ltr even when html dir is rtl #719

Comments

lord007tn commented Feb 21, 2024 • edited Loading

Reproduction

Describe the bug

System Info

Contributes

zernonia commented Feb 22, 2024

lord007tn commented Feb 22, 2024

zernonia commented Feb 23, 2024

Saeid-Za commented Feb 27, 2024 • edited Loading

sadeghbarati commented Feb 27, 2024

Saeid-Za commented Feb 27, 2024 • edited Loading

Update

sadeghbarati commented Mar 1, 2024

zernonia commented Mar 2, 2024

Saeid-Za commented Mar 3, 2024 • edited Loading

Using Wrapper Component (Current Implementation)

Using Vue Plugin

zernonia commented Mar 4, 2024

Saeid-Za commented Mar 5, 2024

zernonia commented Mar 6, 2024

Saeid-Za commented Mar 6, 2024

zernonia commented Mar 6, 2024 • edited Loading

Saeid-Za commented Mar 6, 2024

lord007tn commented Feb 21, 2024 •

edited

Loading

Saeid-Za commented Feb 27, 2024 •

edited

Loading

Saeid-Za commented Feb 27, 2024 •

edited

Loading

Saeid-Za commented Mar 3, 2024 •

edited

Loading

zernonia commented Mar 6, 2024 •

edited

Loading