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] wrong len in draw transition #6195

Merged
merged 3 commits into from
Sep 10, 2021
Merged

[fix] wrong len in draw transition #6195

merged 3 commits into from
Sep 10, 2021

Conversation

wjtje
Copy link
Contributor

@wjtje wjtje commented Apr 14, 2021

Description

This PR add the strokeWith to the len in the draw transistion if the strokeLinecap is 'round' or 'square',
that fixes issue #4540.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@pngwn pngwn added runtime Changes relating to runtime APIs and removed feature: transitions labels Jun 27, 2021
@benmccann benmccann changed the title fix: wrong len in draw transition [fix] wrong len in draw transition Jun 30, 2021
@Conduitry Conduitry merged commit 05953d6 into sveltejs:master Sep 10, 2021
const len = node.getTotalLength();
let len = node.getTotalLength();
if (getComputedStyle(node).strokeLinecap !== 'butt') {
len += parseInt(getComputedStyle(node).strokeWidth);
Copy link
Member

Choose a reason for hiding this comment

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

This was merged, so I'm a little late possibly, but what are the performance implications of calling getComputedStyle twice here? It at least could be deduplicated to one call.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, whoops. It's probably not ideal. I'd definitely merge a PR that just called it once and saved the value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug runtime Changes relating to runtime APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants