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(hideVisually): remove clip-path from hideVisually #454

Merged
merged 2 commits into from Sep 7, 2019
Merged

fix(hideVisually): remove clip-path from hideVisually #454

merged 2 commits into from Sep 7, 2019

Conversation

ughitsaaron
Copy link
Contributor

@ughitsaaron ughitsaaron commented Jul 16, 2019

I came across this bug in Chrome recently, where the use of clip-path forces the browser to make unnecessary repaints. The conventional fix across several other libraries has been to simply fallback to clip while the bug is still unresolved. For instance,

Thanks for all your hard work on this library!

The use of `clip-path` harms performance during scroll events in Chrome. In the meantime, browsers
can fall back to using `clip` until this bug in Chrome is fixed.
@ughitsaaron ughitsaaron requested a review from bhough as a code owner July 16, 2019 02:05
@codecov
Copy link

codecov bot commented Jul 16, 2019

Codecov Report

Merging #454 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #454   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          82     82           
  Lines         565    565           
  Branches      167    167           
=====================================
  Hits          565    565
Impacted Files Coverage Δ
src/mixins/hideVisually.js 100% <ø> (ø) ⬆️

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 b0762e9...507fa46. Read the comment docs.

return {
border: '0',
clip: 'rect(0 0 0 0)',
clipPath: 'inset(50%)',
// clipPath: 'inset(50%)',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to explain the decision here: I chose to comment this out instead of removing the line for posterity and to make it explicit that clip-path is not being used.

@bhough bhough added the bug label Sep 6, 2019
@bhough bhough mentioned this pull request Sep 7, 2019
14 tasks
@bhough bhough changed the base branch from master to version-3-5-release September 7, 2019 13:31
@bhough bhough merged commit b8ef2ba into styled-components:version-3-5-release Sep 7, 2019
@ughitsaaron ughitsaaron deleted the remove-clip-path branch September 25, 2019 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants