superset icon indicating copy to clipboard operation
superset copied to clipboard

fix: useTruncation infinite loop, reenable dashboard cross links on ChartList

Open kgabryje opened this issue 11 months ago • 4 comments

SUMMARY

Due to a bug in useChildElementTruncation, there was an edge case which could trigger an infinite loop of rerenders. The bug occurs when the difference in width of subsequent numbers after the plus sign (e.g. +4 and +5) is just big enough to change the truncation state to the other number. This PR ensures that the calculation is performed only once, and if the edge case is triggered, we simply accept the off-by-one error. This PR also implements a ResizeObserver which recalculates the truncations if the container size changes. Previously we relied on the width values from references in useLayoutEffect's dependency array, which was not a correct approach, since the changes of those values would not trigger a rerender.

This change allowed us to bring back the feature that relied on the truncation logic, which is dashboard cross links on Chart List page. The feature adds a column to the list which displays the dashboards that given chart is added to.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before (occurred long ago when "dashboards added to" column was not yet hidden):

image

After:

image

TESTING INSTRUCTIONS

  1. Visit ChartList
  2. Verify that "Dashboards added to" column is visible and that it correctly displays the dashboards. Dashboard names should be clickable and navigate to given dashboard. Truncated dashboards should appear in a tooltip

ADDITIONAL INFORMATION

  • [ ] Has associated issue:
  • [ ] Required feature flags:
  • [ ] Changes UI
  • [ ] Includes DB Migration (follow approval process in SIP-59)
    • [ ] 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

kgabryje avatar Mar 27 '24 09:03 kgabryje

Codecov Report

Attention: Patch coverage is 97.91667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 69.79%. Comparing base (951d7d6) to head (62932b3).

Files Patch % Lines
...d/components/nativeFilters/FilterCard/ScopeRow.tsx 50.00% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #27701      +/-   ##
==========================================
+ Coverage   69.78%   69.79%   +0.01%     
==========================================
  Files        1911     1914       +3     
  Lines       75024    75103      +79     
  Branches     8355     8390      +35     
==========================================
+ Hits        52352    52415      +63     
- Misses      20622    20624       +2     
- Partials     2050     2064      +14     
Flag Coverage Δ
javascript 57.52% <97.91%> (+0.04%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 27 '24 13:03 codecov[bot]

@justinpark would you be able to do another pass?

kgabryje avatar Apr 03 '24 13:04 kgabryje

/testenv up

kgabryje avatar Apr 03 '24 13:04 kgabryje

@kgabryje Ephemeral environment spinning up at http://34.220.216.100:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

github-actions[bot] avatar Apr 03 '24 13:04 github-actions[bot]

Ephemeral environment shutdown and build artifacts deleted.

github-actions[bot] avatar Apr 09 '24 10:04 github-actions[bot]