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

Pr/44 merge conflicts resolved #2

Merged
merged 15 commits into from
Feb 21, 2019
Merged

Pr/44 merge conflicts resolved #2

merged 15 commits into from
Feb 21, 2019

Conversation

JoniVR
Copy link

@JoniVR JoniVR commented Feb 21, 2019

Hi,
So here's the PR, you'll probably still need to fix some stuff as this was a work in progress.
If you'd rather start fresh on your own than just ignore this ;)

Some things that still need to be fixed (I think):

  • order of the context-menu-options in fixture.js
  • showCopyImageAddress not showing up in fixture.js
  • some actions aren't working properly as far as I can tell (copy for example)

edit:
I accidentally closed the first one

sindresorhus and others added 15 commits December 16, 2017 00:21
sindresorhus#40 eliminated the arguments passed to the click events in both the save image and inspect element context menu items. However the PR description had only referenced issues with the click event in inspect element.

As a result of this change [some apps have lost the save image functionality when passing their browser window](mattermost/desktop#707).

The cause of this failure is the fact that [electron-dl](https://github.com/sindresorhus/electron-dl/blob/master/index.js#L129) requires methods exposed by the `win` provided by the click event.
@JoniVR JoniVR changed the title Pr/44 Pr/44 merge conflicts resolved Feb 21, 2019
@jarivo
Copy link
Owner

jarivo commented Feb 21, 2019

@JoniVR Thanks for the PR! I'll merge it and see if I can fix the remaining issues

@jarivo jarivo merged commit f242922 into jarivo:master Feb 21, 2019
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.

7 participants