search

twbs / bootstrap

The most popular HTML, CSS, and JavaScript framework for developing responsive, mobile first projects on the web.
https://getbootstrap.com
MIT License
170.94k stars 78.89k forks source link

RTL: remaining tasks #32330

Open ffoodd opened 3 years ago

ffoodd commented 3 years ago

Mostly around cheatsheet automation.

Questions

Need help

Docs improvements

Chore

Personal note, as I'm getting exhausted by all those naming/aliasing/restoring utilities discussions:

FaridAghili commented 3 years ago

32327

Great job, noticed this issues so far.

pau1phi11ips commented 3 years ago

Is there a discussion somewhere for the huge amount of work it's going to add to migrations changing l/r to s/e?

ffoodd commented 3 years ago

@pau1phi11ips You may open a discussion if you need; regarding migration, I'd suggest you have a look at our migration guide.

pau1phi11ips commented 3 years ago

I've checked out the migration guide. Moving from 4 to 5 didn't look like it's be that much work until the left/right to start/end bombshell. I was just surprised I couldn't find any existing discussions on the change.

FaridAghili commented 3 years ago

It's still not much work as a couple of find/replace commands will do the work.

It's a huge step forward to support RTL languages since many modern UI frameworks have support for it, it worth the effort.

ffoodd commented 3 years ago

Well @pau1phi11ips, the PR itself is probably the biggest discussed PR in Bootstrap history, and its description links to dozens of previous discussions—with the latest one #30918 helping to choose the way we wanted to implement RTL. It's been announced several times on Twitter and on our blog (in our alpha releases announcements).

I don't know what more we could've done, but any suggestion is welcome since we can improve our docs and communication before we hit stable release, to prevent anyone from being surprised like you were.

This is a major release because of those breaking changes. And as @FaridAghili said, changes are 1:1 for class names, so it's really straightforward.

pau1phi11ips commented 3 years ago

Thanks for the links @ffoodd, that's what I was after in the first place. Sorry I missed the news about this before.

I appreciate having RTL support is good for Bootstrap. Just not sure it's method is the best for what must be over 95% of current Bootstrap 4 users. Maybe memories of updating all our frontend pages from Bootstrap 3 to 4 is making me react more severely to breaking changes in 4 to 5 though 😏 Migrating Bootstrap 3 to 4 took frickin' ages to complete.

arafalov commented 3 years ago

Is it fair to say that the RTL approach chosen is all or nothing? Is it possible to have a website (e.g. CMS-generated) where an article or slideshow content in English is mixed with another in Arabic? I did not dig into this super deep, but the initial documentation seems to indicate that. And, in cheatsheet, I did not see things like bdi element and I did see related issues with elements like "...browse" because of neutral nature of "." character in native input elements.

This may be quite important limitation to help people understand what is actually possible. Mixed content is a messy reality for multilingual organizations, so if you have position (or workarounds or even just thoughts), it would be great to know.

ffoodd commented 3 years ago

@arafalov You're right, our approach (currently) is to have two separate files: one for LTR and one for RTL—meaning you can't use both at the same time. And it seems legit since your document has a main language.

However everything still works, so regarding mixed contents you'd do like you did before: using our utilities to layout things (and dir & lang attributes, obviously). We may add a dedicated example with mixed contents, that could help?

My hope is that logical properties will eventually be supported enough when we'll start v6, so that we drop RTLCSS and separate files. That would solve the mixed content case :)

BTW, are you aware of any tool or framework that handles mixed languages contents in another way? Every source and inspiration is good to know.

arafalov commented 3 years ago

@ffoodd Thank you for the clarification. Yes, I think a dedicated example with mixed content would really help. The bidi is a hard topic, so extra level of clarification early on and demo examples would really simply developers' life and understanding the level of built-in/additional support/effort required.

I am not aware of any other framework doing this better, unfortunately. Drupal does try to support mixed languages content, but that probably does not qualify as framework.

ffoodd commented 3 years ago

@omoridi What issue are you solving? Please open a pull request if you have any patch to submit.

omoridi commented 3 years ago

Hi, why this block is remarked in rtl file?

/ rtl:raw: [type="tel"], [type="url"], [type="email"], [type="number"] { direction: ltr; } /

always these must be ltr

ffoodd commented 3 years ago

@omoridi And this is exactly what it does: ensure direction is ltr even in RTL file. Could you please read our docs, RTLCSS' one and check our RTL cheatsheet example? That way you'll see what's the real result.

omoridi commented 3 years ago

@omoridi And this is exactly what it does: ensure direction is ltr even in RTL file. Could you please read our docs, RTLCSS' one and check our RTL cheatsheet example? That way you'll see what's the real result.

Hi, excuse me for late answering, I write a simple application with .net core, that's convert bootstrap.rtl.css to another file. why? because! 1- You can have rtl and ltr bootstrap objects at same time on the one page! 2- The rtl file too simple and small. .rtl dd { margin-right: 0; } 3- every where if you like you can use rtl class as class='rtl' in body, in div or anywhere else.

I test it, it's okay but, some class must be changed in main bootstrap, where? like you set margin-left and rtlcss changed to marign-right and my simpler application only used marign-right and div have to class margin-left:some value and margin-right:same value. if main bootstrap we must set both value or add some fixer class to fixed that issue.

so what we can? i try to fixed on bootstrap or add helper class for rtl? if you want the rtl sample css, let me say or wait to completed.

ffoodd commented 3 years ago

@omoridi If I understand correctly, you'd need bidi stylesheets, not simply RTL and LTR side-by-side. This is not what we've done currently—and it's explained in another issue's comment. You're the one who opened the said issue, I answered with a precise code sample and solution.

I even added to the current tasks list an item to document this solution—so please consider answering in your previous issue because for now we're only polluting this one.

omoridi commented 3 years ago

@omoridi If I understand correctly, you'd need bidi stylesheets, not simply RTL and LTR side-by-side. This is not what we've done currently—and it's explained in another issue's comment. You're the one who opened the said issue, I answered with a precise code sample and solution.

I even added to the current tasks list an item to document this solution—so please consider answering in your previous issue because for now we're only polluting this one.

Hi, Another problem. .card-link + .card-link { margin-left: 1rem / rtl:ignore /; }

do not ignore it, let changed to margin-right.

ffoodd commented 3 years ago

@omoridi Please open a dedicated issue for bugs, thanks — and please provide a reduced test case with at least some explanations.

omoridi commented 3 years ago

@omoridi Please open a dedicated issue for bugs, thanks — and please provide a reduced test case with at least some explanations.

Let me time, i will be send a test page with .rtl class 😊.

omoridi commented 3 years ago

@omoridi What issue are you solving? Please open a pull request if you have any patch to submit.

Hi, Please check https://github.com/twbs/bootstrap/pull/32807/commits

If you used this method, do not need change any document for rtl!

aqeelat commented 3 years ago

What do you guys think about changing all margins to:

Even though it's a draft, it has good browser support

XhmikosR commented 3 years ago

We don't want to land any further breaking changes (or at least, we want to try to not do it intentionally).

ffoodd commented 3 years ago

@AqeelAT We discussed this possibility while implementing RTL. Logical properties cannot be used throughout our codebase as of now (everything isn't supported yet, e.g. inset property, float logical values, etc.). And we don't want to use them in some places and not others, to keep consistency.

Moreover since we only target RTL support for now, only horizontal direction related properties are considered: there's definetely a nonsense in changing our paradigm for horizontal direction and not the vertical one.

In the renaming part of our RTL implementation (in #31210) we considered logical names too, but we came to the conclusion that it'd make too much breaking changes that aren't mandatory to provide RTL support.

IMHO, v6 (in a few years) will drop RTLCSS and rely on logical properties only. But that's not ready yet for Bootstrap.

aqeelat commented 3 years ago

@ffoodd I mentioned this because in scss/_card.scss:72, the margin does not make sense for rtl. I tried to replace it with margin-inline-start and it worked well. see the itemized card here: https://getbootstrap.com/docs/5.0/examples/cheatsheet-rtl/#card What's another solution for this? I searched for .card-link and it was only in that one file.

ffoodd commented 3 years ago

@AqeelAT There's an RTL directive to ignore this rule — don't remember why. But it's as simple as dropping this ignore rule, so here it is: #33154.

aqeelat commented 3 years ago

Converting Markdown to HTML should surround rtl text with a <bdi dir="rtl"> tag

ffoodd commented 3 years ago

@AqeelAT I'm not sure to understand your last comment, would you have an example where surrounding would be mandatory?

aqeelat commented 3 years ago

@ffoodd For some weird reason, RTL text is arranged from left to right, and then displayed as such. Imagine it this way: the sentence: "hello world" becomes "dlrow olleh" and then written in cursive in this order.

This explains it. link to the page. https://imgur.com/0yNj9n6.png

ffoodd commented 3 years ago

Wow @AqeelAT thanks for finding this, I had no idea! Need to check if Hugo allows some tweaks for such cases, but I'll add this to the to-do list.

Thanks again for your contributions, I'm learning a lot :heart:

omoridi commented 3 years ago

Dir is not good idea! if you want to support right direction use start end properties not left and right. but this is have an issue chrome version 89 and above support it firefox version 66 and above other browser do not support. https://developer.mozilla.org/en-US/docs/Web/CSS/border-end-end-radius

but I like this and I will use it.

SafaAlfulaij commented 3 years ago

Bug: Offcanvas close button doesn't switch place: https://github.com/twbs/bootstrap/blob/a9d7a62658c5d93dcba5ed5fc47d84f3ddd3e0a3/scss/_offcanvas.scss#L25

ffoodd commented 3 years ago

@SafaAlfulaij Would you mind opening a dedicated issue, please? This one is more docs related now. Thanks!

WEBPerformace commented 1 year ago

My hope is that logical properties will eventually be supported enough when we'll start v6, so that we drop RTLCSS and separate files. That would solve the mixed content case :)

Hey hey! Hello from the future! Today logical properties are supported widely enough to finally consider this question! 😉

https://caniuse.com/css-logical-props

aqeelat commented 1 year ago

Would a find-all “margin-left” and replace with “margin-start” work? Or would it be on a case-by-case basis?Anyways, I’d love to be involved.