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.

Sign up for GitHub

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

Jump to bottom

Fix issue #64: Print dialogue only open once in safari on Mac #134

Merged
merged 4 commits into from
Jun 23, 2019
Merged

Fix issue #64: Print dialogue only open once in safari on Mac #134

merged 4 commits into from
Jun 23, 2019

Conversation

eleonora-guidi
Copy link
Contributor

Alternative solution to PR #56

@eleonora-guidi
Fix issue MatthewHerbst#64: Print dialogue only open once in safari o…
6657772 
…n Mac

Removed removeWindow method. Tested on example and custom app, seems to work fine on Chrome, Firefox and Safari.

Updated version on package.json and Changelog file.
@eleonora-guidi
Fix issue MatthewHerbst#64: Print dialogue only open once in safari o…
4a03924 
…n Mac

Added a class to the printing iframe. Always remove the oldest iframe the second time user tries to print (and then append the new one).
Tested on example.
@eleonora-guidi
Fix issue MatthewHerbst#64: Print dialogue only open once in safari o…
f0a6cd5 
…n Mac

Removed console.log
@eleonora-guidi eleonora-guidi mentioned this pull request May 31, 2019
Closed
MatthewHerbst
MatthewHerbst requested changes May 31, 2019
View reviewed changes
Copy link
Owner

@MatthewHerbst MatthewHerbst left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I'll test it out this weekend. One thing: can you please remove the CHANGELOG and version changes? Those will be in their own PR

@eleonora-guidi
Fix issue MatthewHerbst#64: Print dialogue only open once in safari o…
519265a 
…n Mac

Revert changes on changelog and package.json
@eleonora-guidi
Copy link
Contributor Author

Thanks for the PR! I'll test it out this weekend. One thing: can you please remove the CHANGELOG and version changes? Those will be in their own PR

Hi! Thank you, if you have time to test it in the weekend it would be really great!
I have reverted the changes on the changelog and package.jsoon as requested. Hope it's fine.

MatthewHerbst
MatthewHerbst approved these changes Jun 23, 2019
View reviewed changes
Copy link
Owner

@MatthewHerbst MatthewHerbst left a comment

Choose a reason for hiding this comment

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

I've tested this on desktop Chrome and Safari and I've had others test it on mobile Safari on both iPad and iPhone, and it seems to work.

Apologies for the delay - I don't have an iOS device myself so it took a while to get it tested on mobile Safari.

Going to merge and publish, hopefully it solves issues for everyone!

@MatthewHerbst MatthewHerbst merged commit 06c5dd3 into MatthewHerbst:master Jun 23, 2019
@MatthewHerbst
Copy link
Owner

v2.1.3 is published! Please let me know if you run into an other issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MatthewHerbst MatthewHerbst MatthewHerbst approved these changes

Assignees
No one assigned
Labels
bug Safari
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eleonora-guidi @MatthewHerbst