Skip to content

LMS: Adds Language Selection Menu - #2419

Merged
flowerhack merged 25 commits into
masterfrom
talbs/lms-languagemenu
Feb 14, 2014
Merged

LMS: Adds Language Selection Menu#2419
flowerhack merged 25 commits into
masterfrom
talbs/lms-languagemenu

Conversation

@talbs

@talbs talbs commented Feb 2, 2014

Copy link
Copy Markdown
Contributor

This works adds a language selection menu to a user's dashboard (per UI/feature request found here - https://edx-wiki.atlassian.net/wiki/pages/viewpage.action?pageId=48726317).

@talbs

talbs commented Feb 2, 2014

Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn’t there be only the list of languages that are properly (above X%?) translated on transifex?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tusbar, yes. This is my first pass (as a Designer) plumbing through the UI. These values where just to help other team members understand the format we're using and for me to gauge the UI under large list conditions. This will be fully automated and integrated with the platform. :)

Apologies for the confusion.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This list will actually be generated on the server, from the list of actually-supported languages. Our plan is that it will present like this though, with each language presented in its own language.

@talbs

talbs commented Feb 10, 2014

Copy link
Copy Markdown
Contributor Author

@marcotuts, mind reviewing this from a design/FED perspective?

@dianakhuang

Copy link
Copy Markdown
Contributor

@nedbat @sarina

@sarina

sarina commented Feb 12, 2014

Copy link
Copy Markdown
Contributor

screen shot 2014-02-12 at 3 25 47 pm

Can English at least show up in this menu?

Also, maybe here would be a good place to plug Transifex. "Don't see your preferred language? Volunteer to become a translator!" with the second sentence linking to https://github.com/edx/edx-platform/blob/master/docs/en_us/developers/source/i18n_translators_guide.rst

Comment thread common/djangoapps/student/views.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dedent

@sarina

sarina commented Feb 12, 2014

Copy link
Copy Markdown
Contributor

Visiting http://127.0.0.1:8000/dashboard?preview-lang=en@lolcat doesn't break on master but it does on this branch.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is there a way to just define this once in this file in case things change (eg we change how the dummy language is defined)

@sarina

sarina commented Feb 12, 2014

Copy link
Copy Markdown
Contributor

Can you set a few languages that can show up in the dropdown menu in a development environment ("fr", "eo", "ko-kr", "es-419" please)?

@talbs

talbs commented Feb 13, 2014

Copy link
Copy Markdown
Contributor Author

@sarina thanks for the UI suggestion about getting the word out with helping with translating the platform. Sounds reasonable to me - @shnayder, what do you think of this from a product perspective?

@shnayder

Copy link
Copy Markdown

@talbs I like the idea of adding a pointer to the translation volunteer instructions. I also like the idea of adding English to the list...

@talbs

talbs commented Feb 13, 2014

Copy link
Copy Markdown
Contributor Author

@shnayder, thanks! I'll take care of adding the link in. My assumption is that the work that @dianakhuang is doing or @flowerhack has done will populate that select element with the languages the platform supports, including English. Gang, is that assumption correct?

@dianakhuang

Copy link
Copy Markdown
Contributor

@talbs Yes, I'm building out some better default behavior when you don't have any languages 'released' such that English shows up if you don't have anything configured.

@sarina I don't think it makes sense to populate that list in a special way just for development environments. Since that configuration lives in the database, it's not as simple as messing with the lms/envs/dev.py settings file.

Comment thread lms/envs/common.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like we lost this line, which I added recently.

@talbs

talbs commented Feb 13, 2014

Copy link
Copy Markdown
Contributor Author

@sarina, mentioned this in a now outdated diff, but I added translator help text to the modal cases you brought up. I also tried to find any other modals that were in the same situation and sync them up with same help copy.

Comment thread rakelib/bok_choy.rake

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this change in this pull request?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like @wedaly added it to this story to handle dark language things:

edx@adee553

@sarina

sarina commented Feb 13, 2014

Copy link
Copy Markdown
Contributor

@talbs awesome - I know I had no idea what a modal was until just a few months ago.

@dianakhuang ah it was just wishful thinking that dev environments could be set up to test this feature differently. Thanks for the testing notes, I'll poke around some more.

@talbs

talbs commented Feb 13, 2014

Copy link
Copy Markdown
Contributor Author

@dianakhuang, I'm running into a local env devstack issue which looks to be happening due to a server error on the branch - re-provisioning my vagrant machine kicks me back to the master branch as part of its process. When I switch back to this branch and pop my stash of changes, I still receive the server error.

My one outstanding task is to add the link to the translation docs (per @sarina's suggestion). Let me know when it may be safer to try to work locally on this task.

@sarina

sarina commented Feb 13, 2014

Copy link
Copy Markdown
Contributor

Not sure how much we care about this bug but:

  1. Visit http://127.0.0.1:8000/dashboard?preview-lang=eo
  2. Click edit preferred language
  3. Choose English, click Save

When the modal closes, you're still at the url http://127.0.0.1:8000/dashboard?preview-lang=eo so the language still is Esperanto. Removing the ?preview-lang=eo from the URL, then following the steps, works properly.

@sarina

sarina commented Feb 13, 2014

Copy link
Copy Markdown
Contributor

I think it makes sense to force "English" to always be in the list of released languages, as I think this is too easy to mess up. When I do

screen shot 2014-02-13 at 12 31 13 pm

I see

screen shot 2014-02-13 at 12 31 00 pm

@sarina

sarina commented Feb 13, 2014

Copy link
Copy Markdown
Contributor

YIKES also when I do that and select Korean this is what I see in my dashboard afterwards!!!

screen shot 2014-02-13 at 12 32 54 pm

Note I tried this a few languages:
Select French or Esperanto - dashboard properly says my preferred language is French/Esperanto
Select Korean or Español (Latinoamerico) or LOLCAT - incorrectly says my preferred language is English

Note: this is my darklang config now:
eo, fr, ko-kr, es-419, en, en@lolcat

@sarina

sarina commented Feb 13, 2014

Copy link
Copy Markdown
Contributor

is this a bug:

  1. Set preferred language to French
  2. Visit http://127.0.0.1:8000/dashboard?preview-lang=ko-kr - site remains in French

@sarina

sarina commented Feb 13, 2014

Copy link
Copy Markdown
Contributor

@talbs visually it looks odd that there's an icon to the left of email and name but not language:
screen shot 2014-02-13 at 12 39 36 pm

@marcotuts

Copy link
Copy Markdown
Contributor

I'm currently unable to load the LMS through devstack on this branch, though going back to master or other branches works fine. Rake devstack[lms] works, but when I attempt to load the LMS, I see that middleware.py isn't recognized. rake resetdb[devstack] didn't seem to fix this. If anybody has suggestions for how to resolve this let me know! thanks!

@sarina

sarina commented Feb 14, 2014

Copy link
Copy Markdown
Contributor

I left a pile of nitpicky comments about test cases. Overall the code and functionality is well done. 👍 once my concerns addressed.

@flowerhack

Copy link
Copy Markdown
Contributor

Okay, I think i nipped the last of @sarina 's comments in the bud, and bok-choy should be passing now...

(I am getting failures on XBlockAcidTest locally, though? Maybe this needs another rebase @dianakhuang ?)

@sarina

sarina commented Feb 14, 2014

Copy link
Copy Markdown
Contributor

so there are still a good number of pylints https://jenkins.testeng.edx.org/job/edx-platform-report-manual/300/Diff_Quality_Report/?

Can you let me know if these are things that you can fix or should be skipped? If you can fix them please do. If they ought to be skipped and are being erroneously reported I will try to fix in a separate PR. Thanks.

@sarina

sarina commented Feb 14, 2014

Copy link
Copy Markdown
Contributor

Once pylint addressed 🚢 :shipit: 👍 ✨ 🚀 ✈️ ✨ 💯 ✨ 🍇 💚 👻 👹 ✨ ❕ 🔥 🍎 ✨ 🎆 ❇️ 🎇 👾 💖 ✨ ✨ ✨

@dianakhuang

Copy link
Copy Markdown
Contributor

The thing about the pylint violations is that they were all existing violations that were in user_api. They got picked up by diff quality when we moved all of it to common, and as you can see from our discussions with @gwprice , I don't think we understand that code well enough to go through and fix all the docstrings, which is the main pylint violation that we're seeing.

@sarina

sarina commented Feb 14, 2014

Copy link
Copy Markdown
Contributor

Ugh. @gwprice can you put it on your backlog to document that code and fix the violations? It's scary when we have production code that is poorly understood to the point that other developers don't feel like they are capable of adding docstrings.

thanks @dianakhuang . bring this baby to mergetown once commits are nicely squashed

@nedbat

nedbat commented Feb 14, 2014

Copy link
Copy Markdown
Contributor

👍

flowerhack pushed a commit that referenced this pull request Feb 14, 2014
LMS: Adds Language Selection Menu
@flowerhack
flowerhack merged commit 64f1cae into master Feb 14, 2014
@flowerhack
flowerhack deleted the talbs/lms-languagemenu branch February 14, 2014 23:32
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Mar 23, 2018
* Add ORA2 video upload option openedx#2375 (openedx#2417)

* Modified courseware page look and feel openedx#2377 (openedx#2409)

* Modified about page openedx#2379 (openedx#2413)

* Fix biz bugs. openedx#2404 (openedx#2406)

* Fix display width of popup. (openedx#2384)

(cherry picked from commit a43c935a575d15fcf629f0edeb81446c778cda95)

* Validate duplicate url-code.

* Fix bug, when course not found.

* Fix order of course as CourseOverview.

* Remove additional-info count from contract grid. openedx#2419 (openedx#2437)

* Fix order global course. openedx#2420 (openedx#2421)

* Add additional info register. openedx#2419 (openedx#2433)

* fix survey csv character encode problem openedx#2380 (openedx#2434)

* Fix bokchoy for LoginCodeEnabledBizSurveyTest. (openedx#2457)

* Fix register students confirm message. (openedx#2461)

* Add command to check playback_log. openedx#2438 (openedx#2445)

* Fix courseware page lookandfeel (openedx#2446, openedx#2439, openedx#2452, openedx#2453)

* Fix box-shadow of sequence-nav-button. openedx#2453 (openedx#2467)

* Fix bugs. openedx#2462 openedx#2463 (openedx#2464)

* Fix password message in register students page.

* Fix glass pane of processing when register additional item.

* Fix display width of popup. (openedx#2466)

* Fix min-width of sequence-nav. openedx#2468 (openedx#2469)

* Fix isRegistered javascript in about page openedx#2470 (openedx#2471)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants