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 image options type #116 #128

Merged
merged 3 commits into from
Jul 5, 2022
Merged

Conversation

ShaunaTheDead86
Copy link

Issue #116

In line 4 of image.ts, the type ImageOptions is set to be the same as VisualOptions, when it really should be the same as VisualSourceOptions. VisualSourceOptions includes necessary fields for visual layers with an HTML element as its source. We just need to use this type instead of VisualOptions.

@clabe45
Copy link
Collaborator

clabe45 commented Jul 2, 2022

Hello, and thank you for contributing! I think your commit changes a bunch of line endings. Could you please run the following command if you are using Windows:

git config --global core.autocrlf true

and this otherwise:

git config --global core.autocrlf input

And then you'll need to renormalize the line endings before committing again (it's fine to make multiple commits, I can squash them into one when I merge the PR):

git add --renormalize .

@ShaunaTheDead86
Copy link
Author

Thanks! I fixed the line endings. I was wondering why all those files had been changed when I hadn't touched them. I thought maybe the process of running "npm run build" had modified them.

This is my first contribution so I wasn't entirely sure what I was doing! I appreciate the help :)

@clabe45
Copy link
Collaborator

clabe45 commented Jul 3, 2022

No worries at all! It looks great now, except for one last thing. Can you please make a commit that undoes the changes to package-lock.json? They're probably okay, but they might break something down the road.

By the way, that test that failed was broken before your PR, so besides package-lock.json everything looks fine!

@ShaunaTheDead86
Copy link
Author

I believe I reverted the changes to package-lock.json back to the master of the original repo. I don't see it in the changes for this pull request anymore so seems good.

@clabe45
Copy link
Collaborator

clabe45 commented Jul 5, 2022

Looks great! Thanks again :D

Merging

@clabe45 clabe45 merged commit 236a734 into etro-js:master Jul 5, 2022
@clabe45
Copy link
Collaborator

clabe45 commented Jul 5, 2022

By the way, what version of npm are you using? Perhaps I should make a new issue for updating the dependencies in package-lock?

@ShaunaTheDead86
Copy link
Author

Great! :)

And I'm using version 8.11.0 of npm

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.

2 participants