Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Auto-detect "Right To Left" support #2027

Closed
ZvikaZ opened this issue Aug 18, 2016 · 14 comments
Closed

Auto-detect "Right To Left" support #2027

ZvikaZ opened this issue Aug 18, 2016 · 14 comments
Assignees
@yotamberk
Milestone
Minor Release v4.17

Comments

@ZvikaZ
Copy link

ZvikaZ commented Aug 18, 2016 •
edited by mojoaxel
Loading

Hi.

Currently, if the html doc is an RTL (<html dir="rtl">), the timeline isn't rendered 100% correctly - at first glance it looks OK, but as it's zoomed out, the dates are displayed on wrong place.

If it's possible, it'd be great to have the timeline rendered from right to left (to support Hebrew and Arabic) according to dir attribute - i.e., the earliest date on the right and later dates go to the left.

Thanks
@ZvikaZ
Copy link
Author

ZvikaZ commented Aug 19, 2016 •
edited by mojoaxel
Loading

Well, it appears that there is RTL support, but it's just not documented...
Found it in examples section http://visjs.org/examples/timeline/other/rtl.html
But it should be properly documented with all other options.

Furthermore, please consider making the RTL option automatic - derived of the HTML's dir

@mojoaxel mojoaxel added the Docs label Aug 19, 2016
@yotamberk yotamberk mentioned this issue Oct 10, 2016
Merged
@yotamberk
Copy link
Contributor

Hey @ZvikaZ, I'm not sure the rtl should be derived automatically from the HTML's dir.
You might want a left-to-right timeline in a rtl website. Furthermore, I don't think it's fair to enforce rtl with an external attribute.
I did add an appropriate documentation for the rtl option. I hope this satisfies users.

@ZvikaZ
Copy link
Author

ZvikaZ commented Oct 11, 2016

Please read again my original post.
By default, when the timeline is inside an RTL element, it's not rendered as a regular LTR timeline - it's rendered with mistakes...

In the current situation, if the user has an RTL document, and he wants an LTR timeline, he has to wrap it in an LTR div, and don't give the timeline any parameters.
If he wants it to be an RTL timeline, he should give it the correct configuration.
This is cumbersome...

I assume that fixing the initial problem is too complicated, and not every important.

The easiest solution (in my opinion...) is too derive the configuration from the wrapping element.
Anyway, if the user wants an LTR timeline in a RTL doc, he should wrap it in a special LTR DIV .

@yotamberk
Copy link
Contributor

Ok i see what you mean.
It can be done.
Do you want to submit a PR or do you want me to take a shot at it?

בתאריך 11 באוק׳ 2016 8:21 לפנה״צ,‏ "Zvika" notifications@github.com כתב:

Please read again my original post.
By default, when the timeline is inside an RTL element, it's not rendered
as a regular LTR timeline - it's rendered with mistakes...

In the current situation, if the user has an RTL document, and he wants an
LTR timeline, he has to wrap it in an LTR div, and don't give the timeline
any parameters.
If he wants it to be an RTL timeline, he should give it the correct
configuration.
This is cumbersome...

I assume that fixing the initial problem is too complicated, and not every
important.

The easiest solution (in my opinion...) is too derive the configuration
from the wrapping element.
Anyway, if the user wants an LTR timeline in a RTL doc, he should wrap it
in a special LTR DIV .

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2027 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEHquI8Ne2VtgBjHPSZnqxVRd0OLOffIks5qyxzJgaJpZM4JncyK
.

@ZvikaZ
Copy link
Author

ZvikaZ commented Oct 11, 2016

Well, if you can do it it'd be great.
Thanks!

בתאריך 11 באוק׳ 2016 09:20,‏ "yotamberk" notifications@github.com כתב:

Ok i see what you mean.
It can be done.
Do you want to submit a PR or do you want me to take a shot at it?

בתאריך 11 באוק׳ 2016 8:21 לפנה״צ,‏ "Zvika" notifications@github.com כתב:

Please read again my original post.
By default, when the timeline is inside an RTL element, it's not rendered
as a regular LTR timeline - it's rendered with mistakes...

In the current situation, if the user has an RTL document, and he wants
an
LTR timeline, he has to wrap it in an LTR div, and don't give the
timeline
any parameters.
If he wants it to be an RTL timeline, he should give it the correct
configuration.
This is cumbersome...

I assume that fixing the initial problem is too complicated, and not
every
important.

The easiest solution (in my opinion...) is too derive the configuration
from the wrapping element.
Anyway, if the user wants an LTR timeline in a RTL doc, he should wrap it
in a special LTR DIV .

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2027 (comment), or
mute
the thread
<https://github.com/notifications/unsubscribe-auth/
AEHquI8Ne2VtgBjHPSZnqxVRd0OLOffIks5qyxzJgaJpZM4JncyK>
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2027 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADulWqiRhOLBxjUn40uMjVlRvQRHRLzYks5qyyqYgaJpZM4JncyK
.

@yotamberk
Copy link
Contributor

I'll give it a go during the holidays. Chag Sameah! ;)

בתאריך 11 באוק׳ 2016 9:22 לפנה״צ,‏ "Zvika" notifications@github.com כתב:

Well, if you can do it it'd be great.
Thanks!

בתאריך 11 באוק׳ 2016 09:20,‏ "yotamberk" notifications@github.com כתב:

Ok i see what you mean.
It can be done.
Do you want to submit a PR or do you want me to take a shot at it?

בתאריך 11 באוק׳ 2016 8:21 לפנה״צ,‏ "Zvika" notifications@github.com
כתב:

Please read again my original post.
By default, when the timeline is inside an RTL element, it's not
rendered
as a regular LTR timeline - it's rendered with mistakes...

In the current situation, if the user has an RTL document, and he wants
an
LTR timeline, he has to wrap it in an LTR div, and don't give the
timeline
any parameters.
If he wants it to be an RTL timeline, he should give it the correct
configuration.
This is cumbersome...

I assume that fixing the initial problem is too complicated, and not
every
important.

The easiest solution (in my opinion...) is too derive the configuration
from the wrapping element.
Anyway, if the user wants an LTR timeline in a RTL doc, he should wrap
it
in a special LTR DIV .

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2027 (comment),
or
mute
the thread
<https://github.com/notifications/unsubscribe-auth/
AEHquI8Ne2VtgBjHPSZnqxVRd0OLOffIks5qyxzJgaJpZM4JncyK>
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2027 (comment), or
mute
the thread
<https://github.com/notifications/unsubscribe-auth/
ADulWqiRhOLBxjUn40uMjVlRvQRHRLzYks5qyyqYgaJpZM4JncyK>
.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2027 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEHquL_PXxvp6nbR60BFn0EVMXAWexaYks5qyysLgaJpZM4JncyK
.

@mojoaxel
Copy link
Member

closed with #2141

@mojoaxel
Copy link
Member

There is still the issue that <html dir="rtl"> is ignored.
@yotamberk Should we do something about this?

@yotamberk
Copy link
Contributor

Umm it can be easily done. I'm just still not sure what I think about it. I think it should be done instead of the options but I don't know how to document the support of this.

@yotamberk yotamberk self-assigned this Oct 26, 2016
@mojoaxel
Copy link
Member

I think it should be done instead of the options but I don't know how to document the support of this.

I think the "rtl" option should have the default value "auto". If it is set to "auto" we try to guess if the user uses "rtl" e.g. by looking for dir="rtl" in the html- or the root-tag, of maybe even by the browser settings.
If the user forces rtl: false we disable it and if it is set to rtl: true we enable it.

see also

@mojoaxel mojoaxel reopened this Oct 26, 2016
@mojoaxel mojoaxel changed the title Right To Left support Auto-detect "Right To Left" support Oct 26, 2016
@yotamberk yotamberk mentioned this issue Oct 26, 2016
Merged
@mojoaxel
Copy link
Member

mojoaxel commented Oct 27, 2016 •
edited
Loading

Here is wat the specs say:

The directionality of the element is the same as the element's parent element**'s** directionality.

I think we have to check every parent element of the timeline for a direction tag.

@yotamberk
Copy link
Contributor

yotamberk commented Oct 27, 2016 •
edited
Loading

The merge request I've add does this. It takes the nearest parent's dir and uses it (unless specified explicitly in options.rtl)

@mojoaxel
Copy link
Member

mojoaxel commented Oct 27, 2016 •
edited
Loading

The merge request I've add does this. It checks takes the nearest parent's dir and uses it (unless specified explicitly in options.rtl)

Yes, but it the root element has no direction we have to check its parent elements up to the dom-root. Usually the direction is set in the document's root element the <html> tag. See comment.

@mojoaxel mojoaxel reopened this Oct 27, 2016
@yotamberk
Copy link
Contributor

Finally can be closed with #2237

@mojoaxel mojoaxel added this to the Minor Release v4.17 milestone Oct 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@yotamberk yotamberk

Labels
None yet
Projects
None yet
Milestone
Minor Release v4.17
Development

No branches or pull requests
3 participants
@mojoaxel @ZvikaZ @yotamberk