Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(resize): add column resize by cell content #309

Merged
merged 20 commits into from Apr 21, 2021

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Apr 9, 2021

  • a quick description of the problem, by default the auto-resize will call the grid.autosizeColumns() which will try to find all columns in the grid viewport and that works ok with a small grid but as soon as we have many columns, it starts to wrap many words (with ellipsis) and some user might prefer to resize the column by the cell content (to match the content width with the cell width) and that might mean going wider than the grid viewport.
  • so this will be an opt-in feature since it will loop through the entire dataset (by default it will read no more than the first 1000 rows) and find/calculate the width it needs to show the entire text value without word being wrapped, in some cases when having many columns it might make the grid wider than the viewport (in that case the horizontal scroll will appear)

TODOs

  • create POC code
  • add new method in Grid Service
  • add grid option to choose which resize type to use (currently defaults to use only the autosizeColumns)
    • default should still always be to use autosizeColumns for performance reasons
  • add full unit tests coverage
  • add Cypress E2E tests

- a quick description of the problem, by default the auto-resize will call the `grid.autosizeColumns()` which will try to find all columns in the grid viewport and that works ok with a small grid but as soon as we have many columns, it starts to wrap many words (with ellipsis) and some user might prefer to resize the column by the cell content (to match the content width with the cell width) and that might mean going wider than the grid viewport.
- so this will be an opt-in feature since it will loop through the entire dataset (by default it will read no more than the first 1000 rows) and find/calculate the width it needs to show the entire text value without word being wrapped, in some cases when having many columns it might make the grid wider than the viewport (in that case the horizontal scroll will appear)
- the new SlickGrid version has `reRenderColumns` exposed as a public method (it was private before)
@codecov
Copy link

codecov bot commented Apr 9, 2021

Codecov Report

Merging #309 (dc8c2a7) into master (b00c64d) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #309   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          212       212           
  Lines        12752     12838   +86     
  Branches      4363      4403   +40     
=========================================
+ Hits         12752     12838   +86     
Impacted Files Coverage Δ
packages/common/src/extensions/extensionUtility.ts 100.00% <ø> (ø)
packages/common/src/services/shared.service.ts 100.00% <ø> (ø)
...common/src/extensions/checkboxSelectorExtension.ts 100.00% <100.00%> (ø)
...ackages/common/src/extensions/gridMenuExtension.ts 100.00% <100.00%> (ø)
...kages/common/src/extensions/headerMenuExtension.ts 100.00% <100.00%> (ø)
.../src/filter-conditions/filterConditionProcesses.ts 100.00% <100.00%> (ø)
packages/common/src/services/grid.service.ts 100.00% <100.00%> (ø)
packages/common/src/services/gridState.service.ts 100.00% <100.00%> (ø)
.../common/src/services/groupingAndColspan.service.ts 100.00% <100.00%> (ø)
...bundle/src/components/slick-vanilla-grid-bundle.ts 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b00c64d...dc8c2a7. Read the comment docs.

- move code in Grid Service
- add all new Column Definition related to resize
- add all new Grid Options related to resize
- also add all necessary resize by content grid options missing
- this latest commit should be the last one on the feature side, it all works now and calculations are properly tweaked. The only left would be to add unit tests for all of this in future commit
- after doing more UI tests, I found that changing freeze column from header menu & clearing pinning was affecting their positions (in some cases resetting them completely) and was also affecting the resize by content width calculation
- also we should not read `column.width` from `getColumns` because SlickGrid will always populate it with its default internal width (80), so we can't realy on that
- the editor custom formatter was having a lot more space compare to without, this commit fixes it and now they both have the same cell width
- also found the `resizeMaxWidthThreshold` was inversed when comparing against it
- lastly improve performance by skipping column with fixed `width`
@ghiscoding ghiscoding changed the title WIP - feat(resize): add column resize by cell content feat(resize): add column resize by cell content Apr 20, 2021
@ghiscoding ghiscoding merged commit 515a072 into master Apr 21, 2021
@ghiscoding ghiscoding deleted the feat/auto-resize-by-cell-content branch April 21, 2021 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant