1089572 - [Settings] Making the style of the menu item consistent

Status: RESOLVED FIXED

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

Description

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

Some of the clickable menu items do not have an arrow icon and confuse users. Some of them also lack of highlight. We should make the style of all menu items consistent.

Comment 1

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

Attachment: Link to https://github.com/mozilla-b2g/gaia/pull/25536

Hi EJ, the patch basically replace the DOM of the menu items to as the following:
```
  <li>
    <a class="menu-item" href="#">
      <span></span>
    </a>
  </li>
```
Note that all items were with the href attribute to enable the highlight effect.
Could you help review the patch? Thanks.

Comment 2

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

Just found an interesting bug in this patch, plz check my comment on GitHub ! Thanks Arthur.

Comment 3

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

I've addressed the comments. Would you mind review it again? Thanks.

Comment 4

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

It works awesome ! r+

thanks Arthur.

Comment 5

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

Jenny, not sure whether the review should be done by you. Feel free to redirect to the others if needed. Thanks!

Comment 6

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

Zac, would you help review the change that only updates the selector? Thanks!

Comment 7

[Jenny Lee]

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

Hi Arthur, 
Thanks for the great work ;) Per offline discussion, there are a few screens that also needs fixing.

Also adding Helen for ui-review, thanks Helen!

Comment 8

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

EJ, could you help review the newly added commits addressing the issues reported from Jenny? Thanks.

Comment 9

[Zac C (:zac)]

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

r+

Comment 10

[Helen Huang [:HHuang]]

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

Looks good! Thanks for the implement.

Comment 11

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

Noticed another 4 critical panels are broken in this patch, please check my comment on Github, Arthur.

Thanks !

Comment 12

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

I ended up restoring the handling of "data-href" as it requires more change than I expected. Several call settings panels rely one "_openDialog" in |panel_utils.js| to save the changes to the database. Let's refactor this part when dialog manager lands. Please help review the patch again, thanks!

Comment 13

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

Comment on attachment 8511901 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/25536

r+ !! Thanks Arthur, we will fix the data-src problem when settings dialog got finished.

Comment 14

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

Thanks.

master: 1af362224eb9ba09f13ad9c363e1c6fff6044431

Comment 15

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

It breaks simcard manager.

revert: 81dcb3c3f900014975a3b5e9b4645fa58c36bc4b

Comment 16

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

New PR: https://github.com/mozilla-b2g/gaia/pull/25825

Comment 17

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

master: 338e5698029ab015c4e1f45f5a9b948882f2a1d8