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

Refactor Walkontable table renderers #6089

Merged
merged 40 commits into from Jul 25, 2019

Conversation

@budnix
Copy link
Member

commented Jul 5, 2019

Context

This PR refactors Walkontable table renderer. This change introduces new files and code structures, which makes table rendering easier to maintain. In this approach, every TABLE element which can hold children is rendered separately by an individual unit called internally OrderView. A specialized renderer wraps each unit.

Spreading the responsibility of renderers:

<table>
 <colgroup>  \ (root node)
   <col>      \
   <col>       \___ ColGroupRenderer
   <col>       /
   <col>      /
 </colgroup> /
 <thead>     \ (root node)
   <tr>       \
     <th>      \
     <th>       \____ ColumnHeadersRenderer
     <th>       /
     <th>      /
   </tr>      /
 </thead>    /
 <tbody>   ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯\ (root node)
   <tr>   (root node)          \
     <th>  --- RowHeadersRenderer
     <td>  \                     \
     <td>   -- CellsRenderer      \
     <td>  /                       \
   </tr>                            \
   <tr>   (root node)                \
     <th>  --- RowHeadersRenderer     \
     <td>  \                           \___ RowsRenderer
     <td>   -- CellsRenderer           /
     <td>  /                          /
   </tr>                             /
   <tr>   (root node)               /
     <th>  --- RowHeadersRenderer  /
     <td>  \                      /
     <td>   -- CellsRenderer     /
     <td>  /                    /
   </tr>                       /
 </tbody>  ___________________/
</table>

This change helps us to implement an EcoRendering idea in the feature. Currently, the rendering system works the same as in v.7.1.0 with this difference that previously an order of rendered elements (with dataset size 2x3) was TR->TH->TD->TD->TR->TH->TD->TD now TR->TR->TH->TD->TD->TH->TD->TD.

All cells, row headers, and column headers are still rendered on every table render cycle. The performance measurements showed that there are no negative changes. But, surprisingly I've spotted that when viewportColumnRenderingOffset and viewportColumnRenderingOffset are set to 0 new approach works about 20%-30% faster than v7.1.0. (to make this PR non-breaking change this feature was reverted)

How has this been tested?

Tested manually using all known browsers.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

  1. #5769

Checklist:

  • My code follows the code style of this project,
  • My change requires a change to the documentation.

budnix added some commits Jan 23, 2019

WIP
WIP
 * Add new row and column calculator which calculates partially visible
 cells.
 * Temporary disable ecoRenderers
 * Add some tests
Fix all issues indicated by tests
This changes repairs all errors previously indicated by tests that did
not pass. Added support for renderers which shares the same root node
for managing elements. In the table context, it happens only when the
two different instances manage TD elements and TH elements using the
same TH root element. Additionally, fixes some typos, unused variables
and clean up the code.
Implement an old rendering algorithm into a new code structure
Removes the code which was irrelevant for eco rendering strategy. The
eco renderers idea was archived at the "eco-renderers-archive" branch.
Additionally, the documentation of the source code was completed.

@budnix budnix self-assigned this Jul 5, 2019

@budnix budnix changed the title Feature/eco renderers Refactor Walkontable table renderers Jul 8, 2019

budnix added some commits Jul 15, 2019

Add missing code docs and tests
Add unit and e2e tests for table renderers. Add new jasmine matcher
`toBeMatchHTML` which internally uses `pretty` library to better
expectation output format. Updated dev dependency `jest` to the newest
version.
Fix bug while writing the tests
Add tests for missing modules `OrderView` and `SharedOrderView`. While
writing the tests one bug was found which occurs in shared view mode.
The bug which generates the wrong number DOM elements was fixed and
covered with the tests. Additionally, an oversized row heights cache-ing
mechanism was removed from Walkontable - it is already cleared before
table rendering.

budnix and others added some commits Jul 18, 2019

Update src/3rdparty/walkontable/src/utils/column.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>
Update src/3rdparty/walkontable/src/utils/nodesPool.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>
Update src/3rdparty/walkontable/src/utils/orderView/sharedView.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>
Update src/3rdparty/walkontable/src/utils/row.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>
Update src/3rdparty/walkontable/src/renderer/columnHeaders.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>
Update src/3rdparty/walkontable/src/renderer/colGroup.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>
Update src/3rdparty/walkontable/src/utils/column.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>
Update src/3rdparty/walkontable/src/utils/column.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>

@budnix budnix requested a review from swistach Jul 19, 2019

budnix added some commits Jul 19, 2019

Revert slow render behavior
Change of the minimum number of elements at which rendering is
triggered.
Update src/3rdparty/walkontable/src/utils/orderView/sharedView.js
Fix typo

Co-Authored-By: Piotr Laszczkowski <swistach@users.noreply.github.com>

@budnix budnix requested a review from swistach Jul 19, 2019

@swistach
Copy link
Member

left a comment

LGTM 👍

@budnix budnix merged commit 9e8b832 into develop Jul 25, 2019

4 checks passed

continuous-integration/codeship Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
security/snyk - package.json (krzysztofspilka) No new issues
Details

@budnix budnix deleted the feature/eco-renderers branch Jul 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.