hypertrace / hypertrace-ui

UI for Hypertrace
Other
25 stars 12 forks source link

feat: add column config local storage support to table #2417

Closed arjunlalb closed 1 year ago

arjunlalb commented 1 year ago

Remove column config local storage support from table widget renderer. Adapt and add it to table component directly.

codecov[bot] commented 1 year ago

Codecov Report

Merging #2417 (4437af9) into main (24f49c6) will increase coverage by 0.14%. The diff coverage is 97.67%.

@@            Coverage Diff             @@
##             main    #2417      +/-   ##
==========================================
+ Coverage   82.82%   82.96%   +0.14%     
==========================================
  Files         921      921              
  Lines       20549    20557       +8     
  Branches     3241     3245       +4     
==========================================
+ Hits        17020    17056      +36     
+ Misses       3404     3379      -25     
+ Partials      125      122       -3     
Files Changed Coverage Δ
...ojects/common/src/preference/preference.service.ts 95.23% <83.33%> (-1.26%) :arrow_down:
projects/components/src/table/table.component.ts 80.69% <100.00%> (+4.72%) :arrow_up:
...d/widgets/table/table-widget-renderer.component.ts 41.58% <100.00%> (-0.79%) :arrow_down:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   45m 5s :stopwatch: + 2m 5s 1 123 tests ±0  1 112 :heavy_check_mark:  - 11  0 :zzz: ±0  11 :x: +11  1 133 runs  ±0  1 122 :heavy_check_mark:  - 11  0 :zzz: ±0  11 :x: +11 

For more details on these failures, see this check.

Results for commit a66fa19e. ± Comparison against base commit 02e52095.

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   45m 27s :stopwatch: + 2m 27s 1 123 tests ±0  1 105 :heavy_check_mark:  - 18  0 :zzz: ±0  18 :x: +18  1 133 runs  ±0  1 114 :heavy_check_mark:  - 19  0 :zzz: ±0  19 :x: +19 

For more details on these failures, see this check.

Results for commit 9f7ab2e1. ± Comparison against base commit 02e52095.

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   46m 48s :stopwatch: + 3m 48s 1 123 tests ±0  1 105 :heavy_check_mark:  - 18  0 :zzz: ±0  18 :x: +18  1 133 runs  ±0  1 114 :heavy_check_mark:  - 19  0 :zzz: ±0  19 :x: +19 

For more details on these failures, see this check.

Results for commit 7bcde5a4. ± Comparison against base commit 02e52095.

arjunlalb commented 1 year ago

I haven't looked at the full code yet but I have some doubts.

  1. Will we have a way to not set preferences?
  2. Will the preference setting be a default behavior?

The problem that I see here is with the column resizing. We set pixel widths after resizing and if we save everything to the preferences then we will never have a way to revert to the original state in terms of the column sizing. and it will create a problem if someone goes from one screen to another. Should we have a way to reset that as well? I really think we should. We can add an action in the table menu to reset columns the columns.

We are currently saving only two attributes from the column for user preferences. id and the boolean flag visible. Other column properties will continue to work as intended.

arjunlalb commented 1 year ago

Code looks good; no red flags here that I could see. However, table is a beast, so please be sure to test with all forms - flat, expandable, and tree versions. In particular, tree/waterfall. Also explorer table, despite being expandable, works a little different and can be thought of as a fourth version worth testing.

Approved, but please work with @itssharmasandeep on the column resizing concerns he has above as he recently improved that logic and knows it best.

I've tested all the variants you mentioned. LGTM.

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   44m 32s :stopwatch: - 3m 22s 1 123 tests ±0  1 116 :heavy_check_mark:  - 7  0 :zzz: ±0  7 :x: +7  1 133 runs  ±0  1 125 :heavy_check_mark:  - 8  0 :zzz: ±0  8 :x: +8 

For more details on these failures, see this check.

Results for commit 4a0e3697. ± Comparison against base commit 24f49c68.

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   44m 59s :stopwatch: - 2m 55s 1 123 tests ±0  1 116 :heavy_check_mark:  - 7  0 :zzz: ±0  7 :x: +7  1 133 runs  ±0  1 125 :heavy_check_mark:  - 8  0 :zzz: ±0  8 :x: +8 

For more details on these failures, see this check.

Results for commit 9d915e21. ± Comparison against base commit 24f49c68.

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   49m 56s :stopwatch: + 2m 2s 1 126 tests +3  1 119 :heavy_check_mark:  - 4  0 :zzz: ±0  7 :x: +7  1 136 runs  +3  1 128 :heavy_check_mark:  - 5  0 :zzz: ±0  8 :x: +8 

For more details on these failures, see this check.

Results for commit 7bb7ceec. ± Comparison against base commit 24f49c68.

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   46m 3s :stopwatch: - 1m 51s 1 126 tests +3  1 126 :heavy_check_mark: +3  0 :zzz: ±0  0 :x: ±0  1 136 runs  +3  1 136 :heavy_check_mark: +3  0 :zzz: ±0  0 :x: ±0 

Results for commit 4437af94. ± Comparison against base commit 24f49c68.

github-actions[bot] commented 1 year ago

Unit Test Results

       4 files  ±0     310 suites  ±0   54m 18s :stopwatch: + 6m 24s 1 126 tests +3  1 126 :heavy_check_mark: +3  0 :zzz: ±0  0 :x: ±0  1 136 runs  +3  1 136 :heavy_check_mark: +3  0 :zzz: ±0  0 :x: ±0 

Results for commit df5b00c2. ± Comparison against base commit 24f49c68.