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(module:drawer): remove inline style to resolve CSP issue #8065

Merged
merged 1 commit into from Mar 20, 2024

Conversation

arturovt
Copy link
Member

@arturovt arturovt commented Aug 23, 2023

No description provided.

@zorro-bot
Copy link

zorro-bot bot commented Aug 23, 2023

This preview will be available after the AzureCI is passed.

@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.61%. Comparing base (3dc1579) to head (cdac11b).
Report is 38 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8065      +/-   ##
==========================================
- Coverage   91.74%   91.61%   -0.14%     
==========================================
  Files         520      530      +10     
  Lines       18023    18391     +368     
  Branches     2838     2815      -23     
==========================================
+ Hits        16536    16849     +313     
- Misses       1184     1225      +41     
- Partials      303      317      +14     

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

@arturovt arturovt changed the title fix(module:drawer): wrap inline style into binding to resolve CSP issue fix(module:drawer): move style to CSS to resolve CSP issue Feb 3, 2024
@HyperLife1119
Copy link
Collaborator

We should remove --scroll-bar from the drawer component because I can't find any code that is using it.

@arturovt arturovt changed the title fix(module:drawer): move style to CSS to resolve CSP issue fix(module:drawer): remove inline style to resolve CSP issue Mar 14, 2024
@arturovt
Copy link
Member Author

We should remove --scroll-bar from the drawer component because I can't find any code that is using it.

Updated.

Copy link
Collaborator

@Nicoss54 Nicoss54 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work, is it possible to make pass the job failed please ?

After that LGTM

@@ -92,7 +92,7 @@ const NZ_CONFIG_MODULE_NAME: NzConfigKey = 'drawer';
(click)="closeClick()"
aria-label="Close"
class="ant-drawer-close"
style="--scroll-bar: 0px;"
[style.--scroll-bar]="'0px'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: it seems it miss the ". I think that's why the build and test job in the pipeline failed. Maybe

  [style.--scroll-bar]="'0px'"

will be more correct

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's a prettier formatting error.

https://dev.azure.com/ng-zorro/NG-ZORRO/_build/results?buildId=6397&view=logs&jobId=f9e658f5-d354-5a85-a28d-e883c5828737&j=f9e658f5-d354-5a85-a28d-e883c5828737&t=c29d9d40-4b52-5395-542e-4e3bfd14a165

It seems that there are still some problems with zorro's lint-staged. You now need to manually run prettier --fix to format the file. @arturovt

@Nicoss54 Nicoss54 self-requested a review March 14, 2024 17:39
Copy link
Collaborator

@Nicoss54 Nicoss54 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HyperLife1119 HyperLife1119 merged commit 5e89441 into NG-ZORRO:master Mar 20, 2024
8 of 9 checks passed
@arturovt arturovt deleted the fix/drawer-unsafe-inline branch March 20, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants