Skip to content

Conversation

@DivyanshiVashist
Copy link
Contributor

No description provided.


<div id="container">
<div id="target">
<img src="Nyancat.png" height="10%" width="10%">
Copy link
Contributor

Choose a reason for hiding this comment

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

kill the cat

width: 100px;
}
#target {
position: absolute;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think we need this.


var aPositionValue = Number(aPositionValueString);
if (aPositionUnit === '%') {
if (Number(index) === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

To get rid of magic numbers you could possibly set a boolean flag outside of the for loop. E.g var isWidth = true

// TODO: Need to support other positions as currently this only supports positions in which both x and y are specified and are in px
if (analysedPosition.length < 2) {
for (var i = analysedPosition.length; i < 2; i++) {
if (Number(i) === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could maybe use the bool flag idea here again

return undefined;
}
var toParse = [basicShapePolygonParse, basicShapeCircleParse, basicShapeInsetParse, basicShapeEllipseParse];
// var toParse = [basicShapeCircleParse];
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed anymore 😄


circlePathString = offsetPathParse('circle(10px at 50% 50%)', target).path;
assert.equal(circlePathString, 'M 100 50 m 0,-10 a 10,10 0 0,1 10,10 a 10,10 0 1,1 -10,-10 z');

Copy link
Contributor

Choose a reason for hiding this comment

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

Should add a test for garbage input to circle

if (radiusUnit[1] === '%') {
var height = parentProperties.height;
var width = parentProperties.width;
if (!height || !width) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should just be checking !parentProperties.


<script>
var keyframes = [ {offsetPath: 'circle(100px at 500px 500px)', offsetDistance: '0%'},
{offsetPath: 'circle(100px at 500px 500px)', offsetDistance: '100%'}];
Copy link
Contributor

Choose a reason for hiding this comment

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

The demo should test percentages. I suspect it will fail given the dependency you have on the element during initial keyframe parsing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Although that issue is currently hidden by the soon to be removed startKeyframe == endKeyframe optimisation.

Base automatically changed from master to main January 28, 2021 23:52
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.

3 participants