Rewrite custom borders to use SVGs insead of DIVs

[wojciechczerniak]

Description

Refactor DIV based border renderer to SVG one. The main reason behind this change is the performance of Handsontable instance with multiple cell borders. As a side effect, we've fixed a few long-standing issues with custom borders.

Oh, and they are now beautiful:

This was huge and the results are amazing. Multiple tests were added. Tested on all popular web browsers.

Epic. Work was done within smaller issues.

To Do

• [ ] New documentation for SVG borders, including styling
• [ ] Migration guide from DIV borders to SVG ones, including styling/printing
• [ ] Document all the breaking changes for release notes and blog post

Related issues

We should check those

• [x] handsontable/handsontable#4142 Custom border on fixed row
• [ ] ~#4859 Merged Cells + selection causes improper merge cell highlight~ (related to mergeCells)
• [x] handsontable/handsontable#4940 The selection doesn't work with merged cells with fixed overlays
• [x] handsontable/dev-handsontable#48 The fragmentSelection doesn't draw all the borders in some cases
• [ ] ~#5245 More custom border styles~ (feature request, not included)
• [x] handsontable/handsontable#5298 Printing custom borders
• [x] handsontable/handsontable#5568 CustomBorders draws borders of width 2px+ incorrectly
• [x] handsontable/handsontable#6052 Custom borders plugin hook calls grow exponentially with data size
• [ ] ~#6063 Borders after insert row are not movedown~ (related to cellMeta handsontable/handsontable#6274)
• [x] handsontable/handsontable#6064 Custom borders create many elements and operations in DOM
• [ ] handsontable/handsontable#6392 If the table fits well within the viewport, border line is not drawn on an edge of an overlay
• [ ] handsontable/handsontable#6453 Border priority of borders of the same size
• [ ] handsontable/handsontable#6588 iOS: Borders are missing while editing a cell
• [ ] handsontable/dev-handsontable#176 When we select half-shown columns the viewport does not scroll to the exact place to show all borders

Checklist

• [x] Various widths: 1px, 2px, ..., 5px?
• [x] Various colours
• [x] Various combinations, thick with thin, vertical, horizontal
• [x] Is it possible to print the border?
• [x] Border spanned across rows / columns
• [x] Each cell border different
• [ ] Put the performance at test, create as many borders as possible
• [x] Creating/removing borders from context menu
• [x] Creating/removing borders from API
• [x] How does it work with updateSettings?
• [x] Can we change the styles with CSS?
• [x] Do we have prformance lab test?
• [x] How does it look like on Firefox, Safari, Chrome, Edge and IE
• [ ] Performance with and without autoColumnSize
• [ ] With different setting of scroll (System Preferences/General on MacOs)
• [x] scrolling with the fixedBottomRows

[wojciechczerniak]

It should be easier to test the feature/PR with this related issue list and checklist @aninde. If you have anything to add/remove feel free.

CC @AMBudnik @warpech

[AMBudnik]

I would also add

• resizing associated cells (+dbclick - quick resize)
• undo/redo after scrolling /filtering /moving / resizing
• in window scenarios (iframe)

and remember to check all the browsers and go locally ;)

[aninde]

@wojciechczerniak @AMBudnik thank you very much for preparing the checklist. Also, I would like to check custom borders with:

• overlapping customBorders
• intersection customBorders
• fixed/frozen rows/columns
• contextMenu
• dropdownMenu - expanded dropdown list
• autofill
• various overlays - top, left and all
• nested rows and headers
• collapsible columns
• IME (Internationalization)
• on iPad & on mobile

[aninde]

Due to better readability I will report bugs related to svg branch here, not in PR. I use during testing Mac Magic Mouse with the ability to vertical and horizontal scrolling or laptop's trackpad with the same feature I don't click on the scrollbar, I don't want to lost focus on selection.

Case 1

Jumping selection https://github.com/handsontable/handsontable/pull/6157#issuecomment-559401554

Case 2

Leaking fill handle in overlay

Chrome, Safari, Firefox / MacOs Pro, Regression. Use the same setting as in demo https://jsfiddle.net/aninde/f30rjd6u/

Steps to reproduction

1. Select fixed column B.
2. Set viewport to be narrow to see horizontal and vertical scroll.
3. Scroll it horizontally.

Result Notice that the blue square of fileHandle is rectangle, not square and is cutted while scrolling Expected result Should work as in 7.2.2. Should be square and stay with border of fixed column during scrolling.

Case 3

Overlaying right border on frozen columns while scrolling. Occurs on Chrome, Firefox, Opera, Microsoft Dev Edge / MacOs and very fast on safari, so it's hard to notice.

SVG Branch - Firefox 70, locally

7.2.2 Firefox, jsfiddle

Steps to reproduction

1. Set viewport to be narrow to see horizontal and vertical scroll.
2. Select one cell from column C, maybe C11.
3. Scroll it horizontally with changed speed to the right and back to the left the faster when you are close to the end of the left margin of the site.

Result: Pink, the right border is overlaying fixed columns. Selection is doubled and moved. Expected result: Fixed/frozen columns should be on top.

Case 4

Overlaying top border on frozen rows while scrolling.

It works worst in the Firefox, but even in Chrome and Safari doesn't look good.

1. Comment fixedColumnsLeft, uncomment fixedRowsTow in settings.
2. Scroll it vertically with a trackpad, without clicking to change focus.
3. The result is similar to the bug connected to fixedColumnsLeft/frozen columns.

Firefox SVG Branch

Chrome SVG Branch

EDIT: All cases described in this comment also occurs on IE/Real Edge/Firefox on Windows 10. This bug can be reproduced without a trackpad, only with a mouse with scroll-roundel by click and hold on scroll-roundel.

[aninde]

Case 5

Border colors with spaces

customBorders are not rendering in any browser when there is space in color name https://jsfiddle.net/aninde/bfckronw/ 7.2.2 with svg borders

[aninde]

Case 6

Border color order

https://github.com/handsontable/handsontable/issues/6453#issuecomment-560371785 the color order is not preserved.

[aninde]

I'm not sure about the assumption with zooming and blurriness. On PR description was: The borders should looks sharp in every browser at 100% zoom level. They might be blurry at other zoom levels that are not multiplies of 100. I'm not sure that we met that assumption.

I wear glasses so I need someone with healthy eyes to look at it too. @jansiegel @budnix @AMBudnik can you double-check how selection and customBorders looks on zoom 125%, 150% and 200% on Chrome, Firefox, Safari, IE and Microsoft Edge? https://jsfiddle.net/aninde/6c2znt15/2/

Below, I paste print screens on the 150% zoom in and zoom into those print to see details. Sadly the blurriness is worse on Windows. Here is example from Edge.

Chrome/Mac OS 150% zoom

[jansiegel]

@aninde

The borders should looks sharp in every browser at 100% zoom level. They might be blurry at other zoom levels that are not multiplies of 100.

can you double-check how selection and customBorders looks on zoom 125%, 150% and 200% on Chrome, Firefox, Safari, IE and Microsoft Edge?

To me they look sharp at multiplies of 100, and a little blurry on other levels, just as @warpech stated in handsontable/handsontable#6157, at least on Chrome, Firefox and Safari.

[aninde]

Summary of testing

Found:

• [ ] https://github.com/handsontable/handsontable/pull/6157#issuecomment-559401554 Case 1: Jumping selection
• [ ] https://github.com/handsontable/handsontable/issues/6467#issuecomment-559440649 ~Case 2: Leaking fill handle in overlay~ not regression
• [ ] https://github.com/handsontable/handsontable/issues/6467#issuecomment-559440649 Case 3: Overlaying right border on frozen columns while scrolling
• [ ] https://github.com/handsontable/handsontable/issues/6467#issuecomment-559440649 Case 4: Overlaying top border on frozen rows while scrolling
• [ ] https://github.com/handsontable/handsontable/issues/6467#issuecomment-560370346 Case 5: Border colors with spaces
• [ ] https://github.com/handsontable/handsontable/issues/6467#issuecomment-560372097 Case 6: Border color order

Fixed issues that they need to add tests to:

• [ ] handsontable/handsontable#4142 Custom border on fixed row
• [ ] handsontable/handsontable#4940 The selection doesn't work with merged cells with fixed overlays
• [ ] handsontable/handsontable#5298 Printing custom borders with bootstrap css
• [ ] handsontable/handsontable#5568 CustomBorders draws borders of width 2px+ incorrectly
• [x] handsontable/dev-handsontable#48 The fragmentSelection doesn't draw all the borders in some cases

[warpech]

@krzysztofspilka asked me for estimate how much time is needed to finish. Here are my pessimistic estimates (total: up to 9 days).

• https://github.com/handsontable/handsontable/pull/6157#issuecomment-559401554 Case 1: Jumping selection
• https://github.com/handsontable/handsontable/issues/6467#issuecomment-559440649 Case 3: Overlaying right border on frozen columns while scrolling
• https://github.com/handsontable/handsontable/issues/6467#issuecomment-559440649 Case 4: Overlaying top border on frozen rows while scrolling

These are the occurences of the same problem: There is a race condition between border rendering and overlay positioning when main window serves as a scroll container. Additional problem, that seems to be related: in some Walkontable tests, I must call Walkontable.draw() twice to get the desired rendering. Work in progress to fix this is on the branch https://github.com/handsontable/handsontable/commits/feature/issue-6064-touch-overlays, but the further I get, the more I get entangled in quirks of overlay rendering. I might need 2-3 days for this.

• https://github.com/handsontable/handsontable/issues/6467#issuecomment-559440649 Case 2: Leaking fill handle in overlay

This might take 1 day.

• https://github.com/handsontable/handsontable/issues/6467#issuecomment-560370346 Case 5: Border colors with spaces

This might take 1 day.

• https://github.com/handsontable/handsontable/issues/6467#issuecomment-560372097 Case 6: Border color order

This might take 1 day.

Fixed issues that they need to add tests to:

• handsontable/handsontable#4142 Custom border on fixed row
• handsontable/handsontable#4940 The selection doesn't work with merged cells with fixed overlays
• handsontable/handsontable#5298 Printing custom borders with bootstrap css
• handsontable/handsontable#5568 CustomBorders draws borders of width 2px+ incorrectly

Can we please move this out of scope of the PR https://github.com/handsontable/handsontable/pull/6157. My rough estimate is that it might take 1 day of work for each test.

[warpech]

As said in point 2 of https://github.com/handsontable/handsontable/pull/6157 OP, the feature/issue-6064 branch removes the only way to restyle borders using CSS. Meaning: it will no longer be possible to set the border colors of current selection, area selection and fill area using CSS (demo: https://jsfiddle.net/kayw7e20/1/).

I am not sure of the severity of this. Was such possibility ever advertised in the docs? Are our customers using it? If yes, then I think we need to add a proper API for selection customization and explain the migration.

@wojciechczerniak, @krzysztofspilka WDYT?

[aninde]

@warpech the demo in your comment is in the stable version 7.2.2

This is how selection looks on your PR with changed CSS. Only square is changed. Is this in line with the assumptions? 7.2.2. v. with svg, 29.11.19 build. https://jsfiddle.net/aninde/4zkrvqgs/

[warpech]

This is how selection looks on your PR with changed CSS. Only square is changed. Is this in line with the assumptions?

It is in line with my assumptions, yes. I changed the CSS API of the "borders" but did not change the CSS API of the "corners".

[wojciechczerniak]

I am not sure of the severity of this. Was such possibility ever advertised in the docs? Are our customers using it? If yes, then I think we need to add a proper API for selection customization and explain the migration.

It was: https://handsontable.com/docs/1.17.0/tutorial-styling.html and was removed.

I don't have evidence for this, but it might be an important part of the customization process. Everything that is a part of the UI should be customizable in terms of color at least. In handsontable/handsontable#6538 @AMBudnik proposes a feature to keep this feature. I've found one question about it handsontable/handsontable#2487

[warpech]

I don't have evidence for this, but it might be an important part of the customization process. Everything that is a part of the UI should be customizable in terms of color at least.

I made a small PR that implements this: https://github.com/handsontable/handsontable/pull/6593. However there is one pitfall (see in the PR's first comment).

[warpech]

I made a big PR that fixes "Case 1", "Case 3" and "Case 4" explained in this issue: https://github.com/handsontable/handsontable/pull/6594

"Case 5" and "Case 6" were fixed in early December in https://github.com/handsontable/handsontable/pull/6157

@aninde are you sure that "Case 2" is a regression in the SVG borders branch? I think I can reproduce the same problem with 7.2.2 and 7.3.0.

[aninde]

@warpech you are correct. "Case 2" it is not regression, it can be reproduced on 7+ version, my mistake.

[aninde]

New issue explaining a problem discovered in the PR: https://github.com/handsontable/handsontable/issues/6626 (Selection on printed Handsontable)

[aninde]

2. ~The bottom border is doubled, it emerges from under the customBorder. This occurs when fixed rows are enabled.~ EDIT: This is out of Svg-borders scope, because it is not a regression. It's official bug, that I will report. testing 7.3.0.pdf

without fixed rows this bug do not occur. Lack of the bottom border of colHeader is also in version 7.3.0, which is why I don't report it here.

[wojciechczerniak]

Excel does not print them. IMHO, it should not be visible on the print in Handsontable.

+1 on that. We should hide the rest of the selection and white rectangle that covers the grid.

But it's out of the scope so up to you.

Edit: not only custom border is doubled at the bottom:

[warpech]

Keep in mind that the problem still exists that we cannot set custom styles for area selection, until the area selection was used. It is explained in more details in https://github.com/handsontable/handsontable/pull/6593#issue-358600499 (OP and the first comment by @aninde).

Edit: There's a new issue for that: https://github.com/handsontable/handsontable/issues/6656

Edit 2: https://github.com/handsontable/handsontable/issues/6656 is done.

[warpech]

I closed the SVG borders PR https://github.com/handsontable/handsontable/pull/6157, becuase it got largely outdated. At some point, we will reimplement this on a fresh branch.

[adrianszymanski89]

Inform ZD