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

Use of initMouseEvent is deprecated #324

Merged
merged 4 commits into from
Jun 14, 2022

Conversation

Somya-Singhal
Copy link
Contributor

This PR fixed the issue #315 in which I tried to remove the MouseinitEvent as its use is now being deprecated.

@gitpod-io
Copy link

gitpod-io bot commented Apr 6, 2022

@Somya-Singhal Somya-Singhal changed the title Fixed the use of MouseinitEvent in infragram.js Use of initMouseEvent is deprecated Apr 6, 2022
@jywarren
Copy link
Member

jywarren commented Apr 7, 2022

Hi @Somya-Singhal - would it be possible to make the changes in the /src/ folder, and then if we run the command "grunt build" which will compile it with the other files into the /dist/ folder. Does that make sense? Thank you!

@Somya-Singhal
Copy link
Contributor Author

Hi @Somya-Singhal - would it be possible to make the changes in the /src/ folder, and then if we run the command "grunt build" which will compile it with the other files into the /dist/ folder. Does that make sense? Thank you!

Hello @jywarren, as I checked /src/Infragram.js, but in that file, initMouseEvent has not been used anywhere but in the dist/infragram.js it's been used instead. So I can't understand how do I make changes in the /src/Infragram.js first. Please if possible could you elaborate it?

@Somya-Singhal
Copy link
Contributor Author

Hi @Somya-Singhal - would it be possible to make the changes in the /src/ folder, and then if we run the command "grunt build" which will compile it with the other files into the /dist/ folder. Does that make sense? Thank you!

Hello @TildaDares @jywarren, Please could you help me with this.

event = document.createEvent("MouseEvents");
event.initMouseEvent("click", true, true, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
lnk.dispatchEvent(event);
const mouseEvent = getMouseEvent();
Copy link
Member

Choose a reason for hiding this comment

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

Hi @Somya-Singhal sorry for my slow reply as we have a lot of activity from newcomers right now. Basically, these additions can also be made on this line -

https://github.com/Somya-Singhal/infragram/blob/19e01e9b90d75bf3b42552f8450998fde28e57b3/src/io/file.js#L18-L25

And what will happen is you can then run the command grunt build and it will compile your changes and all other files in /src/ into the /dist/ folder. The dist folder is actually auto-generated, we really only edit the /src/ files. So if you can make your changes in /src/ and compile, they'll show up in /dist/. Does that make sense?

Thank you so much!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok @jywarren, I will try this and inform you soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @jywarren, I made changes in /src/file then ran grunt build.After that the following changes were made:
1)In dist/infragram.js

image

image

image

image

image

image

image

image

2)In dist/infragram.js.map
image

  1. In dist/infragram.min.js

image

Had I done something wrong?🙂

Copy link
Member

Choose a reason for hiding this comment

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

That looks right! Can you now git add the file in src, and push it to the same remote branch to add it to this PR? Thank you!

Copy link
Contributor Author

@Somya-Singhal Somya-Singhal Apr 12, 2022

Choose a reason for hiding this comment

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

Can you please review it and let me know if I made any mistake?
I have a doubt that when I pushed it to this same remote branch, it's showing 2 files changed but this time as you said I had made changes only in the src/file, so would it be required to revert the changes that were done in dist/infragram.js in the previous commit or this would work?

event = document.createEvent("MouseEvents");
event.initMouseEvent("click", true, true, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
lnk.dispatchEvent(event);
const mouseEvent = getMouseEvent();
Copy link
Member

Choose a reason for hiding this comment

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

This looks super. Perfectly done with regard to src and dist - thank you!!!! However, I'm now seeing an error when I try to download:

Uncaught ReferenceError: getMouseEvent is not defined
    at Object.downloadImage (infragram.js? [sm]:595:18)
    at Object.download (infragram.js? [sm]:110:18)
    at HTMLButtonElement.onclick ((index):163:107)

Do we need to do something closer to the usage described in https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/MouseEvent ? Or can you link to where you found the solution you used here... maybe there is some typo?

I'm sure we'll solve it and as soon as we do, we can grunt build again and test it out. Thank you @Somya-Singhal !!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the link where I found the solution: naver/billboard.js@dd3b0e4

I think I have to define getMouseEvent() function as specified in this link on line no. 16, that will fix this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @jywarren, I have checked after defining getMouseEvent() function it's now working. I have attached one screenshot below to show that the image is now getting downloaded.
image

Please can you now review it and suggest if any changes are required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @TildaDares @jywarren, Please can you review this pull request?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @jywarren, is there anything I had mistaken in this PR that needs to be changed? I would like to hear your feedback!🙂

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your patience! I just need to try this out and haven't had time, but it looks good! Thanks for your patience as we worked through our backlog with the Outreachy and GSoC programs!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jywarren, for giving constant feedback for helping us out. I would like to work on more issues and will be completing my pending issues by this week.

@@ -2,7 +2,41 @@
// http://github.com/p-v-o-s/infragram-js.

module.exports = function File(options, processor) {

let getMouseEvent = () => {
Copy link
Member

Choose a reason for hiding this comment

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

OK, i tested it in GitPod and got this error:

Uncaught ReferenceError: getMouseEvent is not defined
    at Object.downloadImage (infragram.js:551:30)
    at Object.download (infragram.js:141:22)
    at HTMLButtonElement.onclick (8080-somyasinghal-infragram-vtxgv1qu3w5.ws-us43.gitpod.io/:163:107)

But I looked and I believe you have to recompile the /dist/ files one more time, and/or add them to the pull request with git - your most recent change isn't appearing in /dist/infragram.js. Does that make sense? Thank you and sorry again for the delay!!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, @jywarren for the late response as I was busy with my exams. I have recompiled the /dist/ files and added them to the pull request. I have also tested on my system and found image is getting downloaded. Could you please review it once?

Copy link
Member

@jywarren jywarren left a comment

Choose a reason for hiding this comment

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

Looks great and I tested it successfully in GitPod. Thank you so much for this fix and apologies for the slow response!!

@jywarren jywarren merged commit af3d1dc into publiclab:main Jun 14, 2022
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