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

fix: regression, Row Detail no longer displayed after CSP safe code #1259

Merged
merged 2 commits into from
Dec 9, 2023

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Dec 9, 2023

with the CSP safe code implementation, the SlickRowDetail stopped working because it was using a hack. The hack was to using html string and add closing div tag to fall outside the .slick-cell but now that we use native HTML element then this hack no longer works. What we can do now is to add an optional insertElementAfterTarget that will provide a way to insert our Row Detail container element after our target which is this case is the .slick-cell

TODOs

  • replace HTML string with the use of native HTML element inside SlickRowDetail
  • add proper Cypress E2E tests to make sure that the row detail div is at the correct location in the DOM

before

image

after

image

the issue was because we append everything into the .slick-cell HTML element inside it but SlickRowDetail was hacking the html string to insert the HTML element after the .slick-cell. So our new code will simply do it in the correct way via insertElementAfterTarget

we can see below, the .dynamic-cell-detail div is supposed to go after the .slick-cell.l0.r0 but was instead within it which was wrong and causing the regression
image

Copy link

codecov bot commented Dec 9, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (8f706fc) 86.68% compared to head (b6bc5e2) 86.66%.

Files Patch % Lines
packages/common/src/core/slickGrid.ts 33.34% 2 Missing ⚠️
packages/common/src/services/domUtilities.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next    #1259      +/-   ##
==========================================
- Coverage   86.68%   86.66%   -0.01%     
==========================================
  Files         196      196              
  Lines       21388    21392       +4     
  Branches     7098     7099       +1     
==========================================
+ Hits        18537    18538       +1     
- Misses       2851     2854       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghiscoding ghiscoding merged commit a35f0a4 into next Dec 9, 2023
4 of 5 checks passed
@ghiscoding ghiscoding deleted the bugfix/row-detail-insert-after branch December 9, 2023 21:53
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.

1 participant