handsontable icon indicating copy to clipboard operation
handsontable copied to clipboard

Change the Demo to use a DIV, not an IFRAME

Open warpech opened this issue 3 years ago • 8 comments

Context

This PR changes the demo from using an <iframe> to inserting the demo directly to the page using <div>, <script>, <style>.

The demo remains built from the examples/ and served from examples.handsontable.com.

All framework variants of the page will still render the vanilla JavaScript version, as previously.

How has this been tested?

Tested the JavaScript and React variants locally using npm run docs:dev.

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 to not work as expected)
  • [ ] Additional language file or change to the existing one (translations)

Related issue(s):

  1. #9644

Affected project(s):

  • [ ] handsontable
  • [ ] @handsontable/angular
  • [ ] @handsontable/react
  • [ ] @handsontable/vue
  • [ ] @handsontable/vue3

Checklist:

[skip changelog]

warpech avatar Aug 24 '22 14:08 warpech

Launch the local version of documentation by running:

npm run docs:review 04095a89e77fd57f430347e10fc0b33bc732f997

github-actions[bot] avatar Aug 24 '22 14:08 github-actions[bot]

@warpech, When using the docs:review script, the demo seems to be broken on both vanilla nad react versions:

image Traditionally, doing anything that causes re-render fixes (or partially fixes) the table, but the column headers remain oversized.

Edit: I've built the docs locally (using docs:start), and the demo looks fine there.

jansiegel avatar Aug 25 '22 07:08 jansiegel

The problem with oversized headers reported in https://github.com/handsontable/handsontable/pull/9833#pullrequestreview-1084966930 and https://github.com/handsontable/handsontable/pull/9833#issuecomment-1226863565 is fixed in the commit ae435d72c080b49ff837d498e3d0bcaf2d3262d0. The reason was that Handsontable was initialized before the DOM was fully laid out.

warpech avatar Aug 31 '22 07:08 warpech

I just pushed changes that load the React demo in the React flavor of the docs.

warpech avatar Aug 31 '22 10:08 warpech

@warpech There are quite a lot lint errors and possible styling improvements (lack of empty lines between variable definitions and the logic etc) that need to be addressed before merging.

jansiegel avatar Aug 31 '22 11:08 jansiegel

@warpech There are quite a lot lint errors and possible styling improvements (lack of empty lines between variable definitions and the logic etc) that need to be addressed before merging.

Thanks, I forgot about that. Fixed.

warpech avatar Aug 31 '22 11:08 warpech

@budnix can you please do the re-review, because @jansiegel does not work tomorrow

warpech avatar Sep 01 '22 15:09 warpech

Thank you @aninde but I still want to make the DRY improvements @budnix pointed out.

warpech avatar Sep 08 '22 12:09 warpech

I'll try to finish this PR this week

warpech avatar Oct 26 '22 09:10 warpech

I solved the conflicts created by the fact that this PR was based on handsontable/handsontable#9441, which itself was squashed-merged to develop.

  1. Merge ab2b08e240150e30abdcaf58c4ec8d4ab67993ec (the last commit from handsontable/handsontable#9441 before squashing) into here
  2. Merge 3397ce5ca20a640877710e3f1e935b4106754ee7 (the squash commit from develop) into here
  3. Merge the current develop into here

Doing it in that direction resulted in 1, easy to merge conflict. Any other order resulted in 100+ conflicts.

warpech avatar Oct 31 '22 12:10 warpech

I am working on inlining code of the examples on the demo page, to avoid memory leak problem described by @budnix in his review. However, this task is blocked by https://github.com/handsontable/dev-handsontable/issues/520.

warpech avatar Nov 30 '22 09:11 warpech

Closing in favor of https://github.com/handsontable/handsontable/pull/10138

warpech avatar Dec 02 '22 10:12 warpech