fix(GridState): calling getAssociatedGridColumns should extend column
#1014
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- the previous implementation was extending 3 defined properties but by doing so it was extending properly when a value was provided but it was returning an empty value instead of extending the original
- for example `{ ...gridCol, cssClass: currentCol }` would change the `cssClass` to undefined when `currentCol` doesn't have this `cssClass` and that is even when the original `gridCol` does have that same property. So instead of trying to extend specific properties we should just extend the entire objects, ie: `{ ...gridCol, ...currentCol }` would extend correctly whatever property is defined in `currentCol`
- also make sure to extend currentCol but without the `columnId` since that doesn't exist a regular Column property
Codecov Report
@@ Coverage Diff @@
## master #1014 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 244 244
Lines 16556 16557 +1
Branches 5929 5929
=========================================
+ Hits 16556 16557 +1
|
ghiscoding
added a commit
that referenced
this pull request
Jun 29, 2023
- extends the previous PR #1014, but instead of merging all column properties, we will actually skip the `width` property since that property is changed internally by SlickGrid (auto-calculated) and that has an impact when using autoResizeColumnsByCellContent feature (for example it breaks our Salesforce grid that uses `enabledAutoResizeColumnsByCellContent` and uses much thinner column width, if we however skip the width then we make SlickGrid recalculate the width since we use `enabledAutoResizeColumnsByCellContent` and that fixes our problem)
ghiscoding
added a commit
that referenced
this pull request
Jun 29, 2023
… (part2) (#1015) * fix(GridState): calling `getAssociatedGridColumns` should extend column - extends the previous PR #1014, but instead of merging all column properties, we will actually skip the `width` property since that property is changed internally by SlickGrid (auto-calculated) and that has an impact when using autoResizeColumnsByCellContent feature (for example it breaks our Salesforce grid that uses `enabledAutoResizeColumnsByCellContent` and uses much thinner column width, if we however skip the width then we make SlickGrid recalculate the width since we use `enabledAutoResizeColumnsByCellContent` and that fixes our problem)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
{ ...gridCol, cssClass: currentCol }would change thecssClassto undefined whencurrentColdoesn't have thiscssClassand that is even when the originalgridColdoes have that same property. So instead of trying to extend specific properties we should just extend the entire objects, ie:{ ...gridCol, ...currentCol }would extend correctly whatever property is defined incurrentColcolumnIdsince that doesn't exist a regular Column property