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

Mitigate file Save As issue #1711

Merged
merged 2 commits into from
Feb 28, 2020
Merged

Mitigate file Save As issue #1711

merged 2 commits into from
Feb 28, 2020

Conversation

nikku
Copy link
Member

@nikku nikku commented Feb 28, 2020 •
edited
Loading

A few issues fixed:

  • We append the default file extension
  • We do not fail if the dialog accidentally returns a file without recognizable extension
  • We do not shadow export errors but print them to the console at least

Closes #1699

Please double check if that does not break anything on MacOS (as I believe we removed the extension deliberately during Save As... at some point).

  • Works on Linux
  • Works on MacOS
  • Works on Windows

@nikku
chore(app/dialog): always append default file extension
872ac20 
This ensures that the happy path (saving without changing the file type)
works on Linux.

Related to #1699
@nikku
fix(client): handle unretrievable export config
b51bbc4 
Related to #1699
@nikku nikku requested a review from barmac February 28, 2020 10:32
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Feb 28, 2020
@nikku nikku mentioned this pull request Feb 28, 2020
Closed
barmac
barmac approved these changes Feb 28, 2020
View reviewed changes
Copy link
Contributor

@barmac barmac left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I tested this on Mac and it seems to be fine. Have you tested this on Windows?

Also, I think this could be merged to master once we confirm that it works on all supported platforms.

@nikku nikku changed the base branch from develop to master February 28, 2020 11:01
@nikku
Copy link
Member Author

nikku commented Feb 28, 2020

Did not test on Windows yet.

@pinussilvestrus
Copy link
Contributor

Works fine on Windows 👍

@barmac
Copy link
Contributor

barmac commented Feb 28, 2020

Let's merge it then 🚀

@barmac barmac merged commit 9e3f7a5 into master Feb 28, 2020
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Feb 28, 2020
@delete-merged-branch delete-merged-branch bot deleted the 1699-foo branch February 28, 2020 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@barmac barmac barmac approved these changes

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

Successfully merging this pull request may close these issues.

Impossible to export image
3 participants
@nikku @pinussilvestrus @barmac