936398 - [Settings] Title wording

Status: RESOLVED INVALID

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

Description

[Harly Hsu[:harly]]

Title in settings should follow wording defined in building blocks.

1. Sentence case (Capitalize the first letter of the first word and the first letter of any proper nouns)
2. Limit to one line
3. Forego ending punctuation unless title is a question
4. Try to front load core of message into the first few words

Comment 1

[Harly Hsu[:harly]]

1. Settings: "Call Settings" "Operator Services" should be "Call settings" "Operator services" 
2. Settings->Call Settings: "Call Settings" should be "Call settings"
3. Settings->Sound: "Sent Mail" "Sent Message" should be "Sent mail" "Sent message"
4. Settings->Language: Title "Sample Format" should be "Sample format"
5. Settings->Keyboards->Built-in Keyboard: "Built-in Keyboard" should be "Built-in keyboard"
6. Settings->Keyboards->Selected keyboards->Add more keyboards: "Built-in Keyboard" "Gaia Test Keyboard" should be "Built-in keyboard" "Gaia test keyboard"
7. Settings->Screen lock: "Lock Screen" should be "Lock screen"
8. Settings->Screen lock->Passcode lock: "Create a Passcode" "Confirm Passcode" should be "Create a passcode" "Confirm passcode"
9. Settings->Media storage: "SD Card Storage" should be "SD card storage"
10. Settings->Device information: "Last Updated" should be "Last updated"
11. Settings->Device information->More Information: "More Information" "Build Identifier" should be "More information" "Build identifier"
12. Settings->Device information->Your Rights: "Your Rights" should be "Your rights"
13. Settings->Device information->Your Privacy: "Your Privacy" should be "Your privacy"
8. Settings->Device information->Legal Information: "Legal Information" should be "Legal information"
9. Settings->Improve Firefox OS->Submit Your Feedback: "Submit your Feedback" should be "Submit your feedback"
10. Settings->Operator Services: "Operator Services" should be "Operator services"

Comment 2

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

Set to good first bug.

Comment 3

[Harly Hsu[:harly]]

Add more
11. Settings->Call Settings->Voicemail: "Voice Number" should be "Voice number"
12. Settings->Keyboards->Selected keyboards: "Built-in Keyboard: Number" "Built-in Keyboard: English" should be "Built-in keyboard: Number, Built-in keyboard: English"
13. Settings->Device information->Legal Information->Open Source Notices: "Open Source Notices" should be "Open source notices" 
14. Settings->Device information->Legal Information->Obtaining Source Code: "Obtaining Source Code" should be "Obtaining source code"

Modified
9. Settings->Media storage: "SD Card Storage" should be "SD Card storage"

Comment 4

[Federico 'klez' Culloca]

I'd like to work on this as my first bug.

Let me ask if I'm understanding this corretcly:
In gaia/apps/setting/index.html I see some `li`s containing strings (for example "Call Settings"). Is that what this bug is about?

Comment 5

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

Federico, thank you for willing to contribute to gaia!

Modifying gaia/apps/settings/index.html is only part of the work.
gaia/apps/settings/locale/settings.en-US.properties should also be modified accordingly. And.. that's it!

Let me know if you encounter any problems.

Comment 6

[Federico 'klez' Culloca]

Ok, in ~4 hours I'll be home and I'll take a look. Thanks.

Comment 7

[Federico 'klez' Culloca]

Ok, I had a look inside the gaia/apps/settings tree, and it seems more work than expected.

It will take a bit longer than I thought since a lot of strings changed and were added after this bug was filed, and the html is split between files. Do I have a deadline for this or can I, within limits, take my time?

Comment 8

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

There is no deadline for it. So take your time and enjoy!

Comment 9

[Federico 'klez' Culloca]

Ok, so I added my pull request on github (https://github.com/mozilla-b2g/gaia/pull/14584). Is there anything else I need to do to have it reviewed?

Comment 10

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

Awesome! However, the patch conflicts to the current master. Please rebase it to master. To request a review, please upload an attachment with the following content with a review? flag on reviewers (in the flags section of the upload page). 

<html>
  <head>
  <meta http-equiv="Refresh" content="2; url=https://github.com/mozilla-b2g/gaia/pull/14584">
  </head>
  <body>
    Redirect to pull request 14584
  </body>
</html>

Comment 11

[Federico 'klez' Culloca]

Attachment: pull request

Comment 12

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

Comment on attachment 8346685 [details]
pull request

I noticed that you made the patch on your local master branch, in that way if you pull the latest changes from gaia master, your commits will not be on top of the current gaia master. As you can see there are commits not related to this issue in the pull request. Ideally there is only one commit containing all of the changes you made in the pull request.

Please create another branch of the latest gaia master, and cherry-pick the commits you made. After that, squash the commits into one and create a pull request as you did. Let me know if you encounter any problem. 

Reference: https://developer.mozilla.org/en-US/Firefox_OS/Platform/Gaia/Hacking#Contributing_to_Gaia for details.

Comment 13

[Federico 'klez' Culloca]

I tried doing what you said. I committed to another branch in my fork and did a pull request. Let's hope this time I did everything right...

Comment 14

[Federico 'klez' Culloca]

Attachment: pull request

Comment 15

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

Comment on attachment 8347641 [details]
pull request

We are almost there!

To make a clean commit, we need to squash all commits into one before merging. To do squashing, you need to get the last commit hash before yours by using git log. In the case it seems to be ebae4e59d2d5f12a9094f30818a888099c07991f. And enter the following:

  > git rebase -i ebae4e59d2d5f12a9094f30818a888099c07991f

Then you will be prompted with a text file showing all the commits you made as the following:

  pick <commit hash> <commit message>
  pick <commit hash> <commit message>
  pick <commit hash> <commit message>
  pick <commit hash> <commit message>
  ...

Replace "pick" with "s" or "squash" to remove the commits you don't want. We usually leave the last commit which appears on the first line. So change the text as the following and save:

  pick <commit hash> <commit message>
  s <commit hash> <commit message>
  s <commit hash> <commit message>
  s <commit hash> <commit message>
  ...

After saving the file, you will be prompted with another text file showing the commit messages and followed by a brief description:

  # This is a combination of 4 commits.
  # The first commit's message is:

  xxxxxxxxx

  # The second commit's message is:

  xxxxxx...

  # .....
  # .....
  # .....

Replace the commit messages and leave the description unchanged. In your case it may look like:

  Bug 936398 - fixed settings string to conform to UX guidelines

  # .....
  # .....
  # .....

Save the file and we are done! Check the git log again you will find the commits have been squashed into one. Then we need to "force" push back to github by adding a "-f":

  > git push -f <remote_branch> <branch_name>

git squash reference: http://git-scm.com/docs/git-rebase#_interactive_mode

Comment 16

[Federico 'klez' Culloca]

ok, I'll try as soon as I go home this evening (CET). Thanks for your support.

Comment 17

[Federico 'klez' Culloca]

Is it ok now? Same pull request (14672)

Comment 18

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

There are still two commits in the pull request. Could you squash them into one? Use the steps described in comment 15.

Comment 19

[Federico 'klez' Culloca]

Squashed

Comment 20

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

Please check my comments in github. And it seems conflicts to the current master. After fixing them, the patch is good to go!

Comment 21

[Federico 'klez' Culloca]

I'm having some problems. I tried to rebase my branch to upstream master like this (bug936398 is obviously my local branch)
    
    $ git remote -v
    origin  git://github.com/federicoculloca/gaia.git (fetch)
    origin  git://github.com/federicoculloca/gaia.git (push)
    upstream        [e-mail]:mozilla-b2g/gaia (fetch)
    upstream        [e-mail]:mozilla-b2g/gaia (push)
    $ git fetch upstream
    $ git rebase upstream/master bug936398

This gives me conflicts, but in places that I already corrected.

The problem is that running mergetool (with vimdiff) neither LOCAL nor UPSTREAM seems to be correct. For example on apps/settings/elements/about.html, line 6 I have this:

LOCAL
<h1 data-l10n-id="deviceInfo-header"> Device Information </h1>

BASE
<h1 data-l10n-id="deviceInfo"> Device Information </h1>

REMOTE
<h1 data-l10n-id="deviceInfo"> Device information </h1>

(Notice the uppercase 'I' in LOCAL/Device Information and the wrong id in REMOTE)
While, from my limited understanding of this, I'd expect:

LOCAL
<h1 data-l10n-id="deviceInfo-header"> Device information </h1>

BASE
<h1 data-l10n-id="deviceInfo"> Device Information </h1>

REMOTE
<h1 data-l10n-id="deviceInfo-header"> Device Information </h1>

where LOCAL should be exactly the file I have on my hard drive. What am I not understanding?

I'm very sorry to be doing such a mess...
Also, is bugzilla the right place to discuss such issues or should I take them to the comments on github?

Comment 22

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

Not sure why you got the result you posted. Typically when we want to do rebase, we do the following:

1. $ git checkout master // switch to master
2. $ git pull upstream master // fetch the latest code from upstream and merge it to your local master
3. $ git checkout <your_branch> // switch to the branch that you want to rebase
4. $ git rebase master // rebase to master
5. Git will display conflict if any. Resolve them and do $ git commit

We are using both bugzilla and github. If you would like to comment something on the code, github is a good choice!

Comment 23

[Greg Weng [:snowmantw][:gweng][:λ]]

Since it's far from the last comment, and Wayne want to (and unfortunately had) solve the invalid Bug 944613; Arthur said he could take this bug if the progress is fast and smooth enough.

Comment 24

[Tzu-Wen Lin [:waynux]]

Attachment: Patch

Fix the title wording in settings. it's my first bug!

Comment 25

[Tzu-Wen Lin [:waynux]]

Travis was green.
https://travis-ci.org/mozilla-b2g/gaia/builds/17330636

Comment 26

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

(In reply to Tzu-Wen Lin [:waynux] from comment #25)
> Travis was green.
> https://travis-ci.org/mozilla-b2g/gaia/builds/17330636

Tzh-Wen, sorry to jump in to this thread accidentally (because I am in the cc list XD). 

Based on the comment #1, it looks like this patch only focuses on capitalizing title wordings instead of changing meanings, but don't forget to update related wordings in `settings.en-US.properties`. (I noticed that there are some missing changes in the property file). 

Good job, btw. :)

Comment 27

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

Comment on attachment 8362878 [details] [review]
Patch

Thanks for the patch, Wayne! As EJ mentioned, please also change the corresponding strings in settings.en-US.properties.

Comment 28

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

Comment on attachment 8362878 [details] [review]
Patch

Good work, Wayne. Please check github for a few missing.

And please also update Keyboards -> "Built-in Keyboard".

Comment 29

[Tzu-Wen Lin [:waynux]]

Thank you for detailed reviewing, Arthur and 小龍哥.
I have updated the patch.

Comment 30

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

We are almost there! There are two redundant lines that needs to be removed. Please check my github comments, thanks!

Comment 31

[Tzu-Wen Lin [:waynux]]

The patch is up to date. Thank you for all your assistance.

Comment 32

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

Thanks for the prompt response.

master: 0cad55c5787422ae37c9874b0b2a74080f92c79e

Comment 33

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Awesome work :D

Comment 34

[Axel Hecht [:Pike]]

Arthur, can you do another review pass here? I've came across a few things, and this patch is big enough that I'd rather push this back to you.

Examples:

enabled=Enabled
-> Enable

should have gotten a key change, AFAICT.

turnOn is a button, not a header. Not sure which of "Turn ON", "Turn On" or "Turn on" is actually intended by UX.

Bluetooth might be a proper noun, and not subject to sentence case.

sent-mail and -message aren't titles, not sure what copy guidelines rule them. I actually failed to find the current copy of the copy guidelines :-/

Comment 35

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

Revert per comment 34: adb5bc457e014b6b91c464cefba42e9abe9800ba

And after confirmed with the UX designer, we may change the rule within these weeks. So I would suggest to keep the way as it is now. Mark the bug as invalid.

Comment 36

[[e-mail]]

Harley, could you clarify if the final decision is made for "Turn ON", "Turn On" or "Turn on"?

Comment 37

[Harly Hsu[:harly]]

Hi Fred,
Here are the new rules for writing style guideline for the new Building Blocks:
Title & Sub-title: Title case
Action button: Title case
List button: Title case
List button's sub label: Sentence case
Body content/Dialog body: Sentence case
Value selector: Sentence case

Comment 38

[[e-mail]]

Thanks harly, clear!