1162419 - [gaia-list] every row should be wrap in <li>

Status: RESOLVED WONTFIX

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

Description

[[e-mail]]

current gaia-list row is structured with following style
```
<gaia-list>
  <a>
    <div>
      <h3>Noah Green</h3>
      <p>Atsara Kitchens</p>
      <div class="face"></div>
    </div>
  </a>
</gaia-list>
```

To imply proper semantics, every row should be wrap in `<li>`

Comment 1

[[e-mail]]

FYR,

general link in Settings:

<ul>
<li class="media-storage-section">
  <a class="menuItem-mediaStorage menu-item" data-icon="media-storage" href="#mediaStorage">
    <span data-l10n-id="mediaStorage">Media Storage</span>
    <small class="media-storage-desc menu-item-desc"></small>
  </a>
</li>
</ul>

item with switch in Settings:

<ul>
  <li class="pack-split usb-item">
    <label class="pack-switch">
      <input class="usb-switch" data-ignore type="checkbox" disabled>
      <span></span>
    </label>
    <label id="menuItem-enableStorage" class="name">
      <span class="menu-item" data-icon="usb" data-l10n-id="enableUSBStorage1">USB storage</span>
      <small class="usb-desc menu-item-desc">Disabled</small>
    </label>
  </li>
<ul>

Comment 2

[[e-mail]]

Attachment: pull request redirect to github

wilson & steve,

I made a PR that wrap all list item in <li> tag, and add basic class declaration.

The PR break the compatibility since its force all list item wrap within <li> tag.


```
<li>
  <a>
    <div class="gaia-item-title">Media Storage</div>
    <div class="media-storage-desc menu-item-desc"></div>
  </a>
</li>
```

Comment 3

[[e-mail]]

Sorry the syntax is

```
<li>
  <a>
    <div class="gaia-item-title">Media Storage</div>
    <div class="gaia-item-desc"></div>
  </a>
</li>
```

examples are updated as well

https://github.com/gaia-components/gaia-list/pull/13/files#diff-eacf331f0ffc35d4b482f1d15a887d3bR158

Comment 4

[Steve Chung [:steveck]]

Comment on attachment 8605627 [details] [review]
pull request redirect to github

Some idea is similar to what I proposed in bug 1163921, so it looks fine to me :) (even we might need some backward-capability here). But I might prefer shorter prefix like gi-XXX since it's clear enough for devs that they are applying class from gaia component.

BTW I think we should extend the gaia-list to have more built-in styling instead of living in example only, but we can propose it in other bug anyway.

Comment 5

[Wilson Page [:wilsonpage]]

(In reply to Fred Lin [:gasolin] from comment #2)
> Created attachment 8605627 [details] [review]
> pull request redirect to github
> 
> wilson & steve,
> 
> I made a PR that wrap all list item in <li> tag, and add basic class
> declaration.
> 
> The PR break the compatibility since its force all list item wrap within
> <li> tag.
> 
> 
> ```
> <li>
>   <a>
>     <div class="gaia-item-title">Media Storage</div>
>     <div class="media-storage-desc menu-item-desc"></div>
>   </a>
> </li>
> ```

Is it possible to still get 100% hit-target when <a> is nested inside <li>?

Comment 6

[Wilson Page [:wilsonpage]]

This also has the downside of adding an extra element per list item. For app that have long lists, this can damage performance. What do we favour, symantic or performance? Perhaps we could still support <a> as direct child but somehow use use aria-role attributes to improve semantics.

Comment 7

[[e-mail]]

How about use `data-herf` attribute in <li> to replace the extra <a>?
So there's no extra element per list item and the symantic is improved.


```
<li data-herf="#">
  <div class="gaia-item-title">Media Storage</div>
  <div class="gaia-item-desc"></div>
</li>
```

I'd like to create a gaia-button-like stress test page so we can experiment if some change cause the performance regression.

Comment 8

[[e-mail]]

Run current gaia-list 100 times (with 3 list items)

[mean: 175.29ms of 30], 200.59 | 159.51 | 164.43 | 164.09 | 167.56 | 166.08 | 197.82 | 166.72 | 170.26 | 167.03 | 205.01 | 164.04 | 165.00 | 163.54 | 175.27 | 171.16 | 175.77 | 168.55 | 177.79 | 172.41 | 184.31 | 186.15 | 175.84 | 168.70 | 166.47 | 202.88 | 178.94 | 177.81 | 173.85 | 181.19


Run patched gaia-list 100 times (with 3 list items)

[mean: 195.34ms of 30], 200.79 | 185.16 | 188.71 | 188.50 | 188.90 | 213.77 | 185.61 | 184.45 | 185.72 | 188.63 | 190.18 | 205.01 | 196.71 | 199.58 | 193.62 | 193.30 | 187.30 | 193.90 | 190.92 | 198.06 | 195.23 | 202.58 | 193.59 | 239.94 | 203.81 | 194.43 | 194.74 | 188.47 | 185.70 | 202.96

The performance after patch does slower a bit but not significant, we don't have to sacrifice the symantic for that minor performance gain. But Comment 7 maybe still a better choice.

Comment 9

[[e-mail]]

I forgot to test on mobile, here is the result run on flame 

current
1429.16ms

with patch
1597ms

It has greater impact on mobile device.

Comment 10

[Wilson Page [:wilsonpage]]

(In reply to Fred Lin [:gasolin] from comment #7)
> How about use `data-herf` attribute in <li> to replace the extra <a>?
> So there's no extra element per list item and the symantic is improved.
> 
> 
> ```
> <li data-herf="#">
>   <div class="gaia-item-title">Media Storage</div>
>   <div class="gaia-item-desc"></div>
> </li>
> ```
> 
> I'd like to create a gaia-button-like stress test page so we can experiment
> if some change cause the performance regression.

This looks hacky. The `role` attribute's job is to provide semantic meaning to ambiguous markup [1], let's use it.

<a href="#" role="listitem">
  <div class="gaia-item-title">Media Storage</div>
  <div class="gaia-item-desc"></div>
</a>

[1] http://stackoverflow.com/questions/11131234/the-reason-to-use-role-list-and-role-listitem

Comment 11

[BMO Automation]

Firefox OS is not being worked on