{"action":"created","issue":{"url":"https://api.github.com/repos/palantir/blueprint/issues/1578","repository_url":"https://api.github.com/repos/palantir/blueprint","labels_url":"https://api.github.com/repos/palantir/blueprint/issues/1578/labels{/name}","comments_url":"https://api.github.com/repos/palantir/blueprint/issues/1578/comments","events_url":"https://api.github.com/repos/palantir/blueprint/issues/1578/events","html_url":"https://github.com/palantir/blueprint/pull/1578","id":258977226,"number":1578,"title":"[Table] Improve scroll behavior in TableQuadrantStack","user":{"login":"cmslewis","id":443450,"avatar_url":"https://avatars2.githubusercontent.com/u/443450?v=4","gravatar_id":"","url":"https://api.github.com/users/cmslewis","html_url":"https://github.com/cmslewis","followers_url":"https://api.github.com/users/cmslewis/followers","following_url":"https://api.github.com/users/cmslewis/following{/other_user}","gists_url":"https://api.github.com/users/cmslewis/gists{/gist_id}","starred_url":"https://api.github.com/users/cmslewis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cmslewis/subscriptions","organizations_url":"https://api.github.com/users/cmslewis/orgs","repos_url":"https://api.github.com/users/cmslewis/repos","events_url":"https://api.github.com/users/cmslewis/events{/privacy}","received_events_url":"https://api.github.com/users/cmslewis/received_events","type":"User","site_admin":false},"labels":[{"id":511077827,"url":"https://api.github.com/repos/palantir/blueprint/labels/Domain:%20performance","name":"Domain: performance","color":"5319e7","default":false},{"id":478396745,"url":"https://api.github.com/repos/palantir/blueprint/labels/Domain:%20testing","name":"Domain: testing","color":"d4792f","default":false},{"id":480887520,"url":"https://api.github.com/repos/palantir/blueprint/labels/Package:%20table","name":"Package: table","color":"f5f8fa","default":false},{"id":478396702,"url":"https://api.github.com/repos/palantir/blueprint/labels/Type:%20enhancement","name":"Type: enhancement","color":"f2ca4b","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/palantir/blueprint/milestones/31","html_url":"https://github.com/palantir/blueprint/milestone/31","labels_url":"https://api.github.com/repos/palantir/blueprint/milestones/31/labels","id":2758121,"number":31,"title":"1.29.0","description":"","creator":{"login":"llorca","id":199754,"avatar_url":"https://avatars0.githubusercontent.com/u/199754?v=4","gravatar_id":"","url":"https://api.github.com/users/llorca","html_url":"https://github.com/llorca","followers_url":"https://api.github.com/users/llorca/followers","following_url":"https://api.github.com/users/llorca/following{/other_user}","gists_url":"https://api.github.com/users/llorca/gists{/gist_id}","starred_url":"https://api.github.com/users/llorca/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/llorca/subscriptions","organizations_url":"https://api.github.com/users/llorca/orgs","repos_url":"https://api.github.com/users/llorca/repos","events_url":"https://api.github.com/users/llorca/events{/privacy}","received_events_url":"https://api.github.com/users/llorca/received_events","type":"User","site_admin":false},"open_issues":10,"closed_issues":0,"state":"open","created_at":"2017-09-11T18:05:17Z","updated_at":"2017-09-20T16:14:47Z","due_on":"2017-10-03T07:00:00Z","closed_at":null},"comments":4,"created_at":"2017-09-19T21:57:49Z","updated_at":"2017-09-20T19:49:43Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/palantir/blueprint/pulls/1578","html_url":"https://github.com/palantir/blueprint/pull/1578","diff_url":"https://github.com/palantir/blueprint/pull/1578.diff","patch_url":"https://github.com/palantir/blueprint/pull/1578.patch"},"body":"#### Fixes #1259, Addresses #1547 (insofar as it addresses perf)\r\n\r\n#### Checklist\r\n\r\n- [x] Include tests\r\n\r\n#### Changes proposed in this pull request:\r\n\r\n- 🔄 __CHANGED__ `Table` now debounces quadrant-view sync'ing until ~`250ms`~ `500ms` after scroll has stopped.\r\n    - This means we're doing a _lot_ less DOM accessing per `scroll`/`wheel` event. Now, the only thing we do _during_ the scroll is update the `scroll{Top,Left}` of each quadrant to keep them in sync. This still results in reflows, but we're doing objectively less work per frame now.\r\n    - What now goes into this \"quadrant-view sync'ing\" on scroll _end_ you ask?\r\n        - `Row-header width`: adjusting to fit its widest content (e.g. `1000` is wider than `10`)\r\n        - `Menu-element size`: adjusting to fit the latest row- and column-header sizes.\r\n        - `LEFT/TOP_LEFT quadrant widths`: adjusting to fit the latest row-header width.\r\n        - `TOP/TOP_LEFT quadrant heights`: adjusting to fit the latest column-header height.\r\n        - `LEFT quadrant bottom offset`: adjusting to clear the bottom scrollbar if it exists\r\n        - `TOP quadrant right offset`: adjusting to clear the right scrollbar if it exists\r\n        - [Here](https://github.com/palantir/blueprint/pull/1578/files#diff-72daeac06c3d7ebe4c4ee3cc435e0f2eR614) is the function in the code.\r\n    - ^ All of that used to happen on _every_ `scroll`/`wheel` event. Now it happens only when scroll has stopped.\r\n    - The debounce delay can be disabled for easier unit testing.\r\n- 🐛  __FIXED__ As a result of the above work, scrolling quickly (e.g. clicking and dragging the scroll bar itself) no longer yields row-header width bugs (see: #1259).\r\n\r\nUnder the hood:\r\n- Add `ScrollUtils.measureScrollBarThickness(element: HTMLElement, direction: \"vertical\" | \"horizontal\"`\r\n    - Write unit tests for it.\r\n- Refactor logic in `TableQuadrantStack` to avoid mixing DOM reads and writes\r\n    - Write unit tests for the whole component + fix little bugs I found as a result.\r\n    - Add a caching layer (`TableQuadrantStackCache`) to avoid as many DOM lookups as possible.\r\n- Add unit tests for `TableQuadrant`.\r\n    - Trying to build confidence in our under-the-hood APIs by testing each module in isolation.\r\n\r\n#### Reviewers should focus on:\r\n\r\n- Don't freak out. Most of the LOC changes are in test files.\r\n- @llorca asked about animating the row header to its new width. ~That's seems tricky at a glance, it would be harder to test, and it would add more processing work into the picture. Would rather keep things lean and mean for now, then add that kind of additional processing work once general performance is more solid.~ It's doable with simple CSS, but it's pretty jittery on big tables.\r\n- There is a race condition: the row-header width won't be updated properly if the cells haven't finished rendering before the \"desired\" measurement it done. `500ms` seems like a long enough wait in most cases though.\r\n\r\n#### Screenshot\r\n\r\nThe major visual change here is that row-headers now rejigger to their new \"desired\" size when scrolling has finished.\r\n\r\nThis can be subtle when the header has to expand by only a pixel or so:\r\n![2017-09-19 14 47 39](https://user-images.githubusercontent.com/443450/30617117-895ae6a6-9d49-11e7-91ed-4ff0ab904561.gif)\r\n\r\nOr it can be quite noticeable when the destination number is much wider:\r\n![2017-09-19 14 49 11](https://user-images.githubusercontent.com/443450/30617184-bf80c78c-9d49-11e7-8319-49a4d1a7f173.gif)\r\n![2017-09-19 14 51 47](https://user-images.githubusercontent.com/443450/30617264-1c96255c-9d4a-11e7-85ec-3ed63e5687b8.gif)\r\n \r\nThis behavior is inspired by Excel, which does the exact same thing:\r\n![2017-09-19 14 50 54](https://user-images.githubusercontent.com/443450/30617229-faa86888-9d49-11e7-8589-9fd04eb00fff.gif)\r\n"},"comment":{"url":"https://api.github.com/repos/palantir/blueprint/issues/comments/330961157","html_url":"https://github.com/palantir/blueprint/pull/1578#issuecomment-330961157","issue_url":"https://api.github.com/repos/palantir/blueprint/issues/1578","id":330961157,"user":{"login":"blueprint-bot","id":23267708,"avatar_url":"https://avatars3.githubusercontent.com/u/23267708?v=4","gravatar_id":"","url":"https://api.github.com/users/blueprint-bot","html_url":"https://github.com/blueprint-bot","followers_url":"https://api.github.com/users/blueprint-bot/followers","following_url":"https://api.github.com/users/blueprint-bot/following{/other_user}","gists_url":"https://api.github.com/users/blueprint-bot/gists{/gist_id}","starred_url":"https://api.github.com/users/blueprint-bot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/blueprint-bot/subscriptions","organizations_url":"https://api.github.com/users/blueprint-bot/orgs","repos_url":"https://api.github.com/users/blueprint-bot/repos","events_url":"https://api.github.com/users/blueprint-bot/events{/privacy}","received_events_url":"https://api.github.com/users/blueprint-bot/received_events","type":"User","site_admin":false},"created_at":"2017-09-20T19:49:43Z","updated_at":"2017-09-20T19:49:43Z","author_association":"COLLABORATOR","body":"<h3>Fix tests</h3>\n\nPreview: <strong><a href='https://circleci.com/api/v1/project/palantir/blueprint/3993/artifacts/0//home/ubuntu/blueprint/packages/site-docs/dist/index.html' target='_blank'>documentation</a> | <a href='https://circleci.com/api/v1/project/palantir/blueprint/3993/artifacts/0//home/ubuntu/blueprint/packages/table/preview/index.html' target='_blank'>table</a></strong>\nCoverage: <a href='https://circleci.com/api/v1/project/palantir/blueprint/3993/artifacts/0//home/ubuntu/blueprint/packages/core//coverage/PhantomJS%202.1.1%20%28Linux%200.0.0%29/index.html' target='_blank'>core</a> | <a href='https://circleci.com/api/v1/project/palantir/blueprint/3993/artifacts/0//home/ubuntu/blueprintpackages/datetime//coverage/PhantomJS%202.1.1%20%28Linux%200.0.0%29/index.html' target='_blank'>datetime</a>"}}
{"actor":{"display_login":"blueprint-bot"}}