Skip to content

Adds buttons to the menu to export / import the settings#2545

Open
Ditto-IV100 wants to merge 6 commits intoRocketMap:developfrom
Ditto-IV100:import-export
Open

Adds buttons to the menu to export / import the settings#2545
Ditto-IV100 wants to merge 6 commits intoRocketMap:developfrom
Ditto-IV100:import-export

Conversation

@Ditto-IV100
Copy link
Copy Markdown

@Ditto-IV100 Ditto-IV100 commented Mar 20, 2018

Description

Adds buttons to the menu to export / import the settings.

Motivation and Context

Because the settings are stored in "Local Storage", they can be lost. Not anymore! You can export the settings for a later import.

By default, the file will have the following name: "RocketMap_DD-MM-YYYY HH:mm.txt".

How Has This Been Tested?

I use it on my own server.

Screenshots (if appropriate):

export-import

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Comment thread static/js/map.js Outdated
})
}

function download(name, settings) { // eslint-disable-line no-unused-vars
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

download -> downloadSettings

Comment thread static/js/map.js Outdated
a.click()
document.body.removeChild(a)
}
function upload(e) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

upload -> loadSettings

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(And add a linebreak between this function and the one before.)

Comment thread static/js/map.js Outdated
console.log(fr.result)
upload(fr.result)
}
fr.readAsText(t.files[0])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just use file.target.files[0].

(And remove the declaration of t.)

Comment thread templates/map.html Outdated
</div>
</div>
<div>
<center><button class="settings" onclick="download('RocketMap', JSON.stringify(JSON.stringify(localStorage)))"><i class="fa fa-upload fa-fw"></i>Export Settings</button></center>
Copy link
Copy Markdown
Contributor

@pogo-excalibur pogo-excalibur Mar 21, 2018

Choose a reason for hiding this comment

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

What's the purpose of the double JSON.stringify()?

Copy link
Copy Markdown
Author

@Ditto-IV100 Ditto-IV100 Mar 21, 2018

Choose a reason for hiding this comment

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

You need two JSON.stringify()'s to get the quotes escaped (for loadSettings(e)).

Comment thread templates/map.html Outdated
</div>
</div>
<div>
<center><button class="settings" onclick="downloadSettings('RocketMap', JSON.stringify(JSON.stringify(localStorage)))"><i class="fa fa-upload fa-fw"></i>Export Settings</button></center>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Double JSON.stringify(JSON.stringify( ? Is the double quoting necessary to make it downloadable?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is not necessary to download it, but to deal with the data during import. This code works fine for me. But maybe I have a cleaner solution. I have to test it.

Comment thread static/js/map.js Outdated
function openSettingsFile(file) { // eslint-disable-line no-unused-vars
var fr = new FileReader()
fr.onload = function () {
console.log(fr.result)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove debug.

Comment thread static/js/map.js Outdated
}

function openSettingsFile(file) { // eslint-disable-line no-unused-vars
var fr = new FileReader()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use more descriptive variable names.

Comment thread static/js/map.js Outdated
}

function loadSettings(e) {
var t = JSON.parse(JSON.parse(e))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here, related to my question about the double stringify.

- use Object.assign instead of Object.keys().forEach
- loadSettings() no longer necessary
- double quoting no longer necessary
@Ditto-IV100
Copy link
Copy Markdown
Author

Ditto-IV100 commented Mar 21, 2018

@sebastienvercammen
The code is now smaller without double quoting of JSON.stringify and i have update my description.

Comment thread static/js/map.js Outdated
}

function downloadSettings(name, settings) { // eslint-disable-line no-unused-vars
var a = document.createElement('a')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this one needs a readabillity update -> more descriptive variable name.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants