Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expanding/collapsing the list of MP in vote table really slow #60

Open
henare opened this issue Nov 13, 2015 · 2 comments
Open

Expanding/collapsing the list of MP in vote table really slow #60

henare opened this issue Nov 13, 2015 · 2 comments

Comments

@henare
Copy link
Member

henare commented Nov 13, 2015

Also a problem upstream openaustralia#871 but really a problem with Ukraine.

@equivalentideas
Copy link

This is a problem for us because of the way the script works.

Currently it targets all the table rows for members of the party you want to see, and changes their css height attribute to make them visible. This is slow because their can be ~100 members for a party. It makes the change individually on each member, which is a lot of elements to process and change in the DOM in one go.

Fixing the bug #61 that made it collapse every MP, rather than just the intended ones, has made a big improvement to this. So instead of always acting on 200-350 of elements, it's only targeting the elements in the party, so max about 120. Previously on this page https://tvfy-ukraine.oaf.org.au/divisions/rada/2015-07-14/3167 the collapse function would take 700ms, now it takes about 200ms for the biggest party. (You can see this in js profiling in browser dev tools).

This is a big improvement, but it's still pretty slow. We're currently just using the built in Bootstrap function for this, but I don't think it's intended to target 100+ elements at a time :S

One solution could be to write our own collapsing script, but we'll have to make sure to cover the accessibility wins we get from using the built in approach. We could drawn on http://filamentgroup.github.io/collapsible/demo.html .

Another option would be to wrap the rows that we want to target in another element, and then collapse that. This way you'd only be targeting a single element, which would probably improve performance more. The only element that we could use to wrap rows inside a table is the tbody (according to MDN you are allowed multiple tbody elements inside a table if all tr's are inside tbody's). We'd need to wrap the party headers as well so you'd have:

<table>
<tbody class="party-block-1">
  <tr>
    <td>Party name</td>
  </tr>
</tbody>
<tbody class="member-block-1">
  <tr>
    <td>Party member name</td>
  </tr>
  <tr>
    <td>Party member name</td>
  </tr>
  <tr>
    <td>Party member name</td>
  </tr>
  <tr>
    <td>Party member name</td>
  </tr>
</tbody>
<tbody class="party-block-2">
  <tr>
    <td>Party name</td>
  </tr>
</tbody>
<tbody class="member-block-2">
  <tr>
    <td>Party member name</td>
  </tr>
  <tr>
    <td>Party member name</td>
  </tr>
  <tr>
    <td>Party member name</td>
  </tr>
  <tr>
    <td>Party member name</td>
  </tr>
</tbody>
</table>

Because this is still so slow, particularly on phones, I think trying the markup change is the first port of call.

@equivalentideas
Copy link

As this is now at the same speed as on They Vote For You, I'm going to set this is to post-mvp. It's not nice, but manageable.

@equivalentideas equivalentideas removed this from the MVP milestone Nov 16, 2015
@equivalentideas equivalentideas removed their assignment Nov 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants