You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/07/22 20:22:24 UTC
[GitHub] [superset] Antonio-RiveroMartnez opened a new pull request, #20833: fix(charts): Table Chart doesn't twitch when resizing
Antonio-RiveroMartnez opened a new pull request, #20833:
URL: https://github.com/apache/superset/pull/20833
### SUMMARY
The problem is that when the table is rendering our `useResizeDetector` triggers changes to hide the scrollbar if necessary which causes that width and height from props are always changing and the component enters into an infinite loop. We are detecting the changes and applying the resize only if the new difference (any) is not related to the scrollbar size.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
### TESTING INSTRUCTIONS
1. Visit the "Charts" page and hit "+chart" button
2. Select "Flights" as dataset and "Table" as viz type
3. Toggle to "Raw Records" in query mode
4. Select ORIGIN_AIRPORT and DAY_OF_WEEK for Columns
5. Select AIRLINE[asc] for ordering
6. Create chart
Expected results:
1. The table isn't twitching
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #20833: fix(charts): Table Chart doesn't twitch when resizing
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20833:
URL: https://github.com/apache/superset/pull/20833#issuecomment-1192934588
# [Codecov](https://codecov.io/gh/apache/superset/pull/20833?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#20833](https://codecov.io/gh/apache/superset/pull/20833?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (55421cd) into [master](https://codecov.io/gh/apache/superset/commit/f011abae2b1bbcffc4eddb1a88872dea622693fb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f011aba) will **increase** coverage by `0.01%`.
> The diff coverage is `84.61%`.
```diff
@@ Coverage Diff @@
## master #20833 +/- ##
==========================================
+ Coverage 66.34% 66.35% +0.01%
==========================================
Files 1756 1756
Lines 66747 66769 +22
Branches 7059 7064 +5
==========================================
+ Hits 44282 44306 +24
+ Misses 20667 20663 -4
- Partials 1798 1800 +2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `52.01% <84.61%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/20833?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...tend/plugins/plugin-chart-table/src/TableChart.tsx](https://codecov.io/gh/apache/superset/pull/20833/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtdGFibGUvc3JjL1RhYmxlQ2hhcnQudHN4) | `41.91% <84.61%> (+3.60%)` | :arrow_up: |
| [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/20833/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `51.74% <0.00%> (-2.33%)` | :arrow_down: |
| [...tend/src/explore/components/controls/ViewQuery.tsx](https://codecov.io/gh/apache/superset/pull/20833/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9WaWV3UXVlcnkudHN4) | `72.72% <0.00%> (ø)` | |
| [...src/explore/components/controls/ViewQueryModal.tsx](https://codecov.io/gh/apache/superset/pull/20833/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9WaWV3UXVlcnlNb2RhbC50c3g=) | `75.00% <0.00%> (+1.08%)` | :arrow_up: |
| [...hart-table/src/DataTable/utils/getScrollBarSize.ts](https://codecov.io/gh/apache/superset/pull/20833/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtdGFibGUvc3JjL0RhdGFUYWJsZS91dGlscy9nZXRTY3JvbGxCYXJTaXplLnRz) | `85.71% <0.00%> (+78.57%)` | :arrow_up: |
Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on a diff in pull request #20833: fix(charts): Table Chart doesn't twitch when resizing
Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #20833:
URL: https://github.com/apache/superset/pull/20833#discussion_r931431271
##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -526,6 +541,41 @@ export default function TableChart<D extends DataRecord = DataRecord>(
[setDataMask],
);
+ const handleSizeChange = useCallback(
+ ({ width, height }: { width: number; height: number }) => {
+ setTableSize({ width, height });
+ },
+ [],
+ );
+
+ useLayoutEffect(() => {
+ // After initial load the table should resize only when the new sizes
+ // Are not only scrollbar updates, otherwise, the table would twicth
Review Comment:
Appreciate the comments! This helps SOOOO much.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas merged pull request #20833: fix(charts): Table Chart doesn't twitch when resizing
Posted by GitBox <gi...@apache.org>.
rusackas merged PR #20833:
URL: https://github.com/apache/superset/pull/20833
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org