[fields] Support RTL out of the box

[alexfauquette]

Follow up of #6571

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[mui-bot]

	Messages
📖	Netlify deploy preview: https://deploy-preview-6715--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	612.8	1,124.5	632.4	788.54	186.915
Sort 100k rows	ms	624.2	1,064.1	1,064.1	900.9	164.669
Select 100k rows	ms	151.8	253.7	213	209.48	33.317
Deselect 100k rows	ms	160.1	311	200.2	233.22	61.119

Generated by 🚫 dangerJS against dd181c8

[alexfauquette]

The current strategy is to split the format when there is a space, and consider that as a group of sections we force it to be left to right. In those groups, each section s isolated with direction automatically deduced from its content

For example:

yyyy/MM hh:mm becomes

<ltr>
	<auto>yyyy</auto>/<auto>MM</auto>
</lt>{" "}
<ltr>
	</auto>hh</auto>:<auto>mm</auto>
</lt>

(I added some spaces and line breaks to simplify the reading)

This forces sections to stay in the same position, whatever the content (digit, ltr, or rtl text) is. Moreover, it ensures we can compute the order in which elements are visible.

The management of the right/left arrows probably needs to be redesigned. For now it's a bit nasty because I did not found where to put it

Here is a code sandbox to experiment behavior

date-fns-jalali : full screen | with code

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[flaviendelangle]

@alexfauquette is this ready for review ?
It is not marked as draft but has no review aksed

[alexfauquette]

Suggested change

	export const getSectionOrder = (
	/**
	* Return a description of sections order from left to right.
	**/
	export const getSectionOrder = (

[LukasTy]

Tremendous work on this part. 🚀
RTL was an oversight initially and, I take, a very painful aspect to implement for fields. 🙈 💯

With AdapterDateFnsJalali it seems to work really nicely. 👌

A few general notes:

1. Is this randomly appearing vertical marker an oversight? Or an unavoidable side-effect of using the special characters?

Screen.Recording.2022-11-29.at.23.19.13.mov

2. Is only AdapterDateFnsJalali supported for now or am I doing something wrong?
   In this example both moment libraries are "not working in many ways".

[flaviendelangle]

Looks great !

[alexfauquette]

Is this randomly appearing vertical marker an oversight? Or an unavoidable side-effect of using special characters?

Probably an error when computing the start/end indexes of the selected characters. I will have a look

Is only AdapterDateFnsJalali supported for now or am I doing something wrong?

Yes, I've faced problems with moment-hijri because there is inconsistencies between parse and format. Not sure if it's a bug or misuse. I opened an issue, but no answer: xsoh/moment-hijri#83

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[flaviendelangle]

For example key Arrow Up moved from MM to 01 but not from 01 to 02

We have several problem to support the editing of moment plugins

1. The date-io addMonths and addYear adds Gregorian years instead of Jalaalian one (easy to fix)

2. moment-jalaali consider an incomplete date like 1401/MM/DD to be valid, which breaks everything 😬

I'm trying to see how to fix the 2nd one, already did the 1st one locally

[flaviendelangle]

@alexfauquette I opened a PR on your repo (alexfauquette#7) to try fixing moment-jalaali

[alexfauquette]

I confirm it fixes the problem

[flaviendelangle]

The idea is probably similar on moment-hijri

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[alexfauquette]

The idea is probably similar on moment-hijri

Yes, I will have a look tomorrow and close the issue if it works 👍

@flaviendelangle @LukasTy I've a last concern about this implementation:

For simplicity, the current solution is adding Unicode character in both ltr and rtl. Do you think it worths the effort to avoid adding those invisible characters in ltr mode?

[LukasTy]

For simplicity, the current solution is adding Unicode character in both ltr and rtl. Do you think it worths the effort to avoid adding those invisible characters in ltr mode?

IMHO—fields implementation is already very complex as-is. I'd lean towards leaving this behaviour in both modes. Besides, it might allow us to spot any possible future issues, which would possibly be missed (because we wouldn't be working with rtl often). 🙈 🤔

[flaviendelangle]

For simplicity, the current solution is adding Unicode character in both ltr and rtl. Do you think it worths the effort to avoid adding those invisible characters in ltr mode?

We already have invisible characters in LTR for the numeric editing anyway.
I would be in favor to keep all the invisible characters and add a small section in "Testing caveats" on how to clean the input value (maybe we could export a testing utility).

[alexfauquette]

On moment-hijri it's a distinct problem.

The adapter also have the problem of adding gregorian months and not hijri ones. But their is a bigger problem linked to the library itself.

Let assume you have a field with YYYY/02/DD

When you press the key UP on months it roughly does the following:

const initialMonth = utils.parse('02', 'iMM');
const newMonth =utils.addMonths(initialMonth, 1);
const new SectionValue = utils.format(newMOnth, 'iMM' );

The problem, with moment-hijri when you do format(parse( '02', 'iMM', isStrict)) You can either get:

• 12 if isStrict is false
• Invalid date if isStrict is true

The example is in the issue I opened

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[flaviendelangle]

@alexfauquette how about adding a warning on the doc for moment-hijri and in the adapter ?
To be able to move on with this topic and at least have a good behavior for Jalali adapters.

[alexfauquette]

Good for me