Skip to content

Initial v2 infinite load implementation#289

Merged
oliverchang merged 7 commits intogoogle:masterfrom
rzhw:frontend3-infiniteload
Feb 6, 2022
Merged

Initial v2 infinite load implementation#289
oliverchang merged 7 commits intogoogle:masterfrom
rzhw:frontend3-infiniteload

Conversation

@rzhw
Copy link
Copy Markdown
Contributor

@rzhw rzhw commented Feb 4, 2022

This PR introduces an initial "infinite load" (as opposed to "infinite scroll") implementation on the vulnerability table, replacing the previous placeholder pagination.

It does not attempt to fix other known issues with the table e.g. columns aren't complete, width goes out of bounds, etc.

79q4QkjzVejBbhQ

Notes

This implementation adds no additional custom JS by taking advantage of <turbo-frame>, part of the pre-existing dependency on Turbo, which replaces only itself—and not the full page—with an updated version upon request load.

For example, the HTML in /list?page=1 looks like the following:

<turbo-frame id="vulnerability-page-1">
  vuln 1
  vuln 2
  ..
  vuln n
  <turbo-frame id="vulnerability-page-2">
    <a href="/list?page=2">Load more</a>
  </turbo-frame>
</turbo-frame>

Similarly, the HTML in /list?page=2 looks like the following:

<turbo-frame id="vulnerability-page-2">
  vuln 1
  vuln 2
  ..
  vuln n
  <turbo-frame id="vulnerability-page-3">
    <a href="/list?page=3">Load more</a>
  </turbo-frame>
</turbo-frame>

Note how everything is nearly the same, except ids attached to the <turbo-frame>.

When clicking the link on page 1, this is intercepted by <turbo-frame id="vulnerability-page-2">, which fetches /list?page=2 and looks for a frame with the same id. It then replaces itself with those contents.

Hence the resulting rendered HTML of:

<turbo-frame id="vulnerability-page-1">
  vuln 1
  vuln 2
  ..
  vuln n
  <turbo-frame id="vulnerability-page-2">
    vuln 1
    vuln 2
    ..
    vuln n
    <turbo-frame id="vulnerability-page-3">
      <a href="/list?page=3">Load more</a>
    </turbo-frame>
  </turbo-frame>
</turbo-frame>

Ignoring the existence of <turbo-frame>, we can also see how this HTML would enable navigation even in web browsers without custom elements support.

Known issues

  • Unfortunately, <table> does not support <turbo-frame> wrapping table rows. This is invalid HTML. Adopting this solution was only possible by first rewriting the vulnerability table using <div> and <span> elements, and using CSS to display it as a table, and ARIA markup for proper screen reader support.
    • In order for the <turbo-frame> to not affect the structure of the table, we take advantage of display: contents; so that the rendered structure of the page is as if all table rows had the same parent.
  • The case of the last page isn't handled.

Further reading

rzhw added 7 commits February 2, 2022 15:14
this is horrific but is required to wrap rows in <turbo-frame>, as it is invalid html to have something wrapping <tr>s inside a <tbody>/<table> context
@rzhw rzhw marked this pull request as ready for review February 4, 2022 09:38
@rzhw rzhw requested a review from oliverchang February 4, 2022 09:38
Copy link
Copy Markdown
Collaborator

@oliverchang oliverchang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thanks!!

@oliverchang oliverchang merged commit 87464ee into google:master Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants