Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Bidi support#2357

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
gnestor merged 20 commits intojupyter:masterfromAshamandi:master
Sep 15, 2017
Merged

Bidi support#2357

gnestor merged 20 commits intojupyter:masterfromAshamandi:master
Sep 15, 2017

Conversation

@samarsultan
Copy link
Contributor

@samarsultansamarsultan commentedApr 2, 2017
edited
Loading

Hello@Carreau ,

This pull request contains the bidi work and fixes the UI mirroring issues.

Here are some functions added such : LoadLocale -> to load the required locale that is defined atMomentjs based on the browser preferences setting . I propose to leave it to the user selection rather than the browser preferences and to create a UI component to toggle among the list of locales.

ForNumeric Shaping : I need another component to toggle between the options.

With respect toNational Calendar , I found an external plugin at Momentjs calledmoment-hijri that is dedicated to Hijri-calendar so I removed the related functions that were mentioned at the design. Actually I didn't findmoment at the documentation . I'll put the links below :

Could you please review ?
Waiting your comments. Thanks !

@samarsultansamarsultan changed the titleBidi supportBidi support #2178Apr 2, 2017
@samarsultansamarsultan changed the titleBidi support #2178Bidi supportApr 2, 2017
@blink1073
Copy link
Contributor

This approach looks great! Having a user toggle for locale makes sense to me.

@blink1073
Copy link
Contributor

pinging@ellisonbg for design consideration.

@samarsultan
Copy link
ContributorAuthor

Could you please review ?

@gnestor
Copy link
Contributor

@samarsultan Can you please resolve the merge conflicts for this PR?

What is the current status? We are closing in on 5.1 release and if this is complete, then let's try to get it in there!

@samarsultan
Copy link
ContributorAuthor

@gnestor ,Conflicts have been resolved.

The current status is having a mirrored UI with loading the dedicated Locale form momentJS.
For Numeric Shaping , I left it to the browser language ,instead of user selection, while loading the locale. So if the language is "ar" , you will see the Arabic locale is loaded and numbers are displayed in Arabic-Indic format.

@gnestor
Copy link
Contributor

@samarsultan Excellent!! How do I set locale or enable mirroring?

@samarsultan
Copy link
ContributorAuthor

For now, you can do it by just setting the browser language to "ar" . However, it will be better to have a user toggle for locale as proposed above.

@gnestor
Copy link
Contributor

Doesn't seem to be working for me. Here's what my Chrome settings look like:

image

The notebook and dashboard are rendering LTR as usual...

@samarsultan
Copy link
ContributorAuthor

You have also to ensure that "Display Google Chrome in this Language" is checked and probably you need to relaunch your browser.

chrome language setting

Or you can simply do it from Firefox by just moving up the Arabic language. It will be easier to return it back to English.

firefox language

@gnestor
Copy link
Contributor

gnestor commentedAug 2, 2017
edited
Loading

Ok, it's working in Firefox. It looks great to me!

It looks like we can include this in 5.1 release assuming that there are no objections.@Carreau@minrk Would you care to review?

@gnestor
Copy link
Contributor

I'm going to mark this as 5.2 and will merge into master as soon as 5.1 is released. This will everyone a chance to use it before it's released.

@gnestorgnestor modified the milestones:5.2,5.1Aug 3, 2017
@gnestor
Copy link
Contributor

5.1 is released so let's merge this and start testing it!

@gnestorgnestor merged commitaa3c1a5 intojupyter:masterSep 15, 2017
if(_isMirroringEnabled()){
$("body").attr("dir","rtl");
}
requirejs(['components/moment/locale/'+_uiLang()], function (err){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ithink we need to special caseen anden-us since the files are not there and are the builtins from moment:moment/moment#2081

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks like you already took care of it! 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Oh yeah I should have noted that again here. Good to go now.

@gnestor
Copy link
Contributor

Notebook 5.2.0rc1 is available on PyPI so please give it a try 👍:

pip install notebook --pre --upgrade

@samarsultan
Copy link
ContributorAuthor

Hello@gnestor and@rgbkrk.

Thanks for reviewing our code.
Could you please update the "README" file and the documentation with the way of enabling the above features?

If you have any comments,@ragabo will take over.

@gnestor
Copy link
Contributor

@takluyver Should we add the following fromhttps://github.com/jupyter/notebook/blob/master/notebook/i18n/README.md#how-the-language-is-selected- to the README?

jupyter notebook command reads the LANG environment variable at startup, (xx_XX or just xx form, where xx is the language code you're wanting to run in).
Hint: if running Windows, you can set it in PowerShell with ${Env:LANG} = "xx_XX".

The preferred language for web pages in your browser settings (xx) is also used. At the moment, it has to be first in the list.

@takluyver
Copy link
Member

I'd focus on getting the machinery for building and contributing translations in place first.

@takluyver
Copy link
Member

I've opened a couple of issues for that -#3102,#3103

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@rgbkrkrgbkrkrgbkrk left review comments

@minrkminrkAwaiting requested review from minrk

@CarreauCarreauAwaiting requested review from Carreau

+1 more reviewer

@gnestorgnestorgnestor approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

5 participants

@samarsultan@blink1073@gnestor@takluyver@rgbkrk

[8]ページ先頭

©2009-2025 Movatter.jp