wireui / wireui

TallStack UI components
https://v1.wireui.dev
MIT License
1.37k stars 166 forks source link

support rtl #169

Closed swarakaka closed 1 year ago

swarakaka commented 2 years ago

In this pull request, components are matched to the right to left direction.

swarakaka commented 2 years ago

hello @putera, could you please merge my PR?

PH7-Jack commented 2 years ago

@swara-mohammed Thanks for your contribution. In soon I will merge your pull request, I'm testing it now

swarakaka commented 2 years ago

Hallo @PH7-Jack Have you set up a review plan? So I ask because I use it in a project.

PH7-Jack commented 2 years ago

@swara-mohammed I'm reviewing this PR, but I need to make sure if all are working. I soon I will reply for you

PH7-Jack commented 2 years ago

@swara-mohammed Thanks for your interest to contribute! I'm so happy with it. The RTL support is a delicate feature because to implement it as well, we need to review some concepts about the RTL

How the calendar is rendered in RTL?

The WireUI uses the right-icon, right-label, and the left-icon in some components, it was thought in the LTR view. We need to discuss if it needs to be changed.

A breaking change: All developers will need to add the current direction in your blade layout.

<!DOCTYPE html>
<html dir="ltr or rtl"

I'm thinking about this feature. I will be so big

putera commented 2 years ago

@swara-mohammed Thanks for your interest to contribute! I'm so happy with it. The RTL support is a delicate feature because to implement it as well, we need to review some concepts about the RTL

How the calendar is rendered in RTL?

The WireUI uses the right-icon, right-label, and the left-icon in some components, it was thought in the LTR view. We need to discuss if it needs to be changed.

A breaking change: All developers will need to add the current direction in your blade layout.

<!DOCTYPE html>
<html dir="ltr or rtl"

I'm thinking about this feature. I will be so big

I agreed with @PH7-Jack. It will be a big changes. I suggest @swara-mohammed can create a new branch for RTL and we can start contribute from there. ❤️

H4ck3r-x0 commented 2 years ago

Hello There @putera @swara-mohammed @PH7-Jack A quick fix is to wrap your component inside a div and add a dir="ltr or rtl"

<div dir="ltr">
     <x-select :options="['اعلى تقييما', 'ممتاز جدا']" wire:model.defer="model"  />
</div>

Not ideal but fix it for me. ;)

MohmmedAshraf commented 1 year ago

@PH7-Jack @swarakaka Hi There it's a great feature btw I used to customize Wireui views to use this concept too it's a headache especially if you use Wireui in many projects that need to be multilanguage, am thinking of a solution without breaking changes but also required a lot of work to be done,

What about a new config option to set the app direction like so

<?php

use WireUi\View\Components;

return [
    'direction' = 'ltr',
]

Then we need to relay on this on each component like @H4ck3r-x0 suggestion:

<div dir="{{ $direction }}">
     <!-- component -->
</div>

So when we set the locale in the middleware or whatever concept the developer use to achieve this also need to set the Wireui direction and we can mention this in the docs.

<?php

namespace App\Http\Middleware;

use Closure;
use Carbon\Carbon;
use Illuminate\Support\Facades\App;
use Illuminate\Support\Facades\Date;

class SetLocaleMiddleware
{

    private function setLocale(string $locale): void
    {
            // Set app language
            App::setLocale($locale);
            Date::setLocale($locale);
            Carbon::setLocale($locale);

            // Set the language direction bassed on the code
            if ($locale == 'ar' || $locale == 'fa' || $locale == 'he' || $locale == 'ku' || $locale == 'ps' || $locale == 'ur') {
                $direction = 'rtl';
            } else {
                $direction = 'ltr';
            }

            config()->set('wireui.direction', $direction);
    }

    public function handle($request, Closure $next)
    {
        if (session()->has('locale')) {
            $this->setLocale(session('locale'));
        }

        return $next($request);
    }
}

What about this @swarakaka I can make it works if you don't have time or if u need any help I can... just ask, @PH7-Jack I couldn't find any other cleaner way to do this but am open to suggestions.

PH7-Jack commented 1 year ago

@PH7-Jack @swarakaka Hi There it's a great feature btw I used to customize Wireui views to use this concept too it's a headache especially if you use Wireui in many projects that need to be multilanguage, am thinking of a solution without breaking changes but also required a lot of work to be done,

What about a new config option to set the app direction like so

<?php

use WireUi\View\Components;

return [
    'direction' = 'ltr',
]

Then we need to relay on this on each component like @H4ck3r-x0 suggestion:

<div dir="{{ $direction }}">
     <!-- component -->
</div>

So when we set the locale in the middleware or whatever concept the developer use to achieve this also need to set the Wireui direction and we can mention this in the docs.

<?php

namespace App\Http\Middleware;

use Closure;
use Carbon\Carbon;
use Illuminate\Support\Facades\App;
use Illuminate\Support\Facades\Date;

class SetLocaleMiddleware
{

    private function setLocale(string $locale): void
    {
            // Set app language
            App::setLocale($locale);
            Date::setLocale($locale);
            Carbon::setLocale($locale);

            // Set the language direction bassed on the code
            if ($locale == 'ar' || $locale == 'fa' || $locale == 'he' || $locale == 'ku' || $locale == 'ps' || $locale == 'ur') {
                $direction = 'rtl';
            } else {
                $direction = 'ltr';
            }

            config()->set('wireui.direction', $direction);
    }

    public function handle($request, Closure $next)
    {
        if (session()->has('locale')) {
            $this->setLocale(session('locale'));
        }

        return $next($request);
    }
}

What about this @swarakaka I can make it works if you don't have time or if u need any help I can... just ask, @PH7-Jack I couldn't find any other cleaner way to do this but am open to suggestions.

It's interesting, but I believe that it's not complete to add the RTL support, but it's a good way to add fast the RTL support

MohmmedAshraf commented 1 year ago

@PH7-Jack Cool I will try to PR this soon thanks.

PH7-Jack commented 1 year ago

I'm closing this PR for now (it's outdated). We can create a discussion for that to point out the most important things.