937970 - [Settings] [Language] Sample Format on top of language selector button seems inappropriate

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Harly Hsu[:harly]]

Attachment: language.png

Advise to move Sample format below the language selector button.

Comment 1

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #13678 - Move 'Sample Format' section below language picker

Comment 2

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Screenshot after patch

This is how the Languages panel from Settings will look like after the patch is applied.

Comment 3

[Harly Hsu[:harly]]

Awesome, it is much better this way as the user can select the language first and then sees the differences in sample format between different languages.
Thanks

(In reply to Mihai Cirlanaru [:mihai][:mcirlanaru] from comment #2)
> Created attachment 831923 [details]
> Screenshot after patch
> 
> This is how the Languages panel from Settings will look like after the patch
> is applied.

Comment 4

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 831922 [details]
Pull Request #13678 - Move 'Sample Format' section below language picker

Thanks for the patch! r=me.

Comment 5

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Thanks Arthur! Landed on master:
https://github.com/mozilla-b2g/gaia/commit/74b70a1ca8efca4facd8ebcd4cbd03bfaad0a6c2

Comment 6

[Carsten Book [:Tomcat]]

reopen, i had to revert this checkin in https://github.com/mozilla-b2g/gaia/commit/ec4d2cb31459ad6e7a4ba7fd3bb49177a110d1d9 since it was breaking Gaia  unit tests

Comment 7

[Zac C (:zac)]

Hi Mihai, with the HTML change you broke a locator in the test's app object.
The locator is here:
https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/apps/settings/regions/language.py#L11

You can re-create a pull with a repaired locator and only merge when Travis is Green for gaia-ui-tests.

Comment 8

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

(In reply to Zac C (:zac) from comment #7)
> Hi Mihai, with the HTML change you broke a locator in the test's app object.
> The locator is here:
> https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/
> gaiatest/apps/settings/regions/language.py#L11
> 
> You can re-create a pull with a repaired locator and only merge when Travis
> is Green for gaia-ui-tests.

Ohhh, sorry about that! Will send a new PR that updates the locator. Thanks for pointing it out!

Comment 9

[Zac C (:zac)]

Yeah sorry, the existing locator is a bit crap. an ID on one of the tags there would be very useful and you wouldn't have his this issue :(

Comment 10

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #13726 - Move 'Sample Format' section below language picker

New pull request that takes care of updating the UI test as well. Let me know if it looks good.

Comment 11

[Zac C (:zac)]

Comment on attachment 832970 [details]
Pull Request #13726 - Move 'Sample Format' section below language picker

f+ from me. Green Travis++

Comment 12

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 832970 [details]
Pull Request #13726 - Move 'Sample Format' section below language picker

r=me. Thanks!

Comment 13

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Landed on master:
https://github.com/mozilla-b2g/gaia/commit/eeda629704666e2645c5aa11303ab10615ff637f

Comment 14

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Comment on attachment 832970 [details]
Pull Request #13726 - Move 'Sample Format' section below language picker

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Settings > Language layout
[User impact] if declined: Low/None
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low/None (moved one section below the other)
[String changes made]: No