-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a metadata tag to override notebook direction (ltr/rtl) #5052
Conversation
notebook/static/notebook/js/main.js
Outdated
try{ | ||
requirejs(['custom/custom'], function() {}); | ||
bidi.loadLocale(); | ||
} catch(err) { | ||
console.log("Error processing custom.js. Logging and continuing"); | ||
console.warn(err); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important note:
I've moved these lines to the very end of the file, because the function bidi.loadLocale
now reads metadata from the notebook and must be called after loading it. The problem is that I don't know if this change could cause any unwanted side effects, especially since there's a line that loads a custom script and I don't know what it is at all.
Please just make sure I'm not breaking things here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
custom.js
is meant as a place for users to put arbitrary javascript to customise the notebook interface. Which probably means any change to how it's loaded is going to break something for someone, somewhere, but that can't really be helped.
I'm not sure why loading it is tied to bidi support, though. :-/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @m2-farzan, I suggest we break this up: leave the custom.js
loading where it is and move the bidi.locale()
handling to the end of the file. This PR also needs a rebase.
Thanks! There's some relevant discussion at ipython/ipython#10980 . Someone has suggested ignoring the browser locale settings and always using LTR layout by default. That would still be compatible with having a direction stored in the notebook metadata, I think. (I can't read any RTL languages, so I'm hoping that a consensus will appear among people who can about the best ways to handle them) |
59da182
to
30ed681
Compare
So I've been working on this recently, and I hope you like the end result. 30ed681 will allow the users to control the direction of the notebook by the following means:
The behavior is described below:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Marking this for 6.2 since we're in a 6.1 release cycle.
I kicked the CI jobs |
Thanks for fixing the CI! |
It would be great if we could also have this in Lab. |
@zareami10 I've been working on it recently. It will be ready soon. |
Currently, the direction of text flow in a notebook (left-to-right / right-to-left) is determined by browser language settings. If the browser is set to use a language with right-to-left script, all notebooks are displayed in RTL direction and vice versa.
This behavior could be improved, IMHO (I'm a student from the middle east). I write my homework in my mother language, but I also download stuff from the internet which are mostly in English. This means that I have to change my browser language settings frequently, which is not user friendly.
This PR makes it possible to override this default behavior per notebook using a metadata tag. Here's how to use this tag in order to make a notebook RTL:
"direction": "rtl"
or to force notebook to use LTR:
"direction": "ltr"
The end user could either manually set this tag or use the
toggle rtl layout
command from the command palette to change the direction of a notebook permanently, without needing to change the browser settings.That's it from the end user point of view.