save map coordinates in local storage by vanithaak · Pull Request #1225 · publiclab/Leaflet.DistortableImage

vanithaak · 2022-10-23T16:08:35Z

Fixes #998 (<=== Add issue number here)

Co-authored by: @7malikk

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

PR is descriptively titled 📑 and links the original issue above 🔗
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt test
code is in uniquely-named feature branch and has no merge conflicts 📁
screenshots/GIFs are attached 📎 in case of UI updates
@mention the original creator of the issue in a comment below for help or for a review

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

GIF

MapKnitter.Lite.-.Google.Chrome.2022-10-22.23-17-57.mp4

…leImage

gitpod-io · 2022-10-23T16:08:43Z

vanithaak · 2022-10-23T16:09:46Z

@jywarren , @TildaDares please review our pr

vanithaak · 2022-10-25T17:03:09Z

@jywarren , please review this pr, not #1220

jywarren · 2022-10-30T18:29:32Z

    json.avg_cm_per_pixel = this._getAvgCmPerPixel(json.images);
-
+    var jsonImages = json.images;
+    savetoLocalStorage(jsonImages);


Hi, this is looking like a good start, thank you! I wanted to suggest that you build on #1237, because it is almost the same! We'll want to make a function which runs each time there is an update to an image... it could even be each time there's a MouseUp, maybe? And it'll write a new "saved state" into localStorage, and similarly to #1237, not just the selected ones but all the images.

Shall we wait a little bit until #1237 is resolve then? Thanks!

Actually wait, cancel that. My mistake. I meant #1161 -- where we are working with the generateExportJson() function. Unfortunately I think there is some overlap between #1237 and #1161, and I'd like to ask you and @leilayesufu to try to work together on #1237 and to record a joint contribution. Would you both be willing to try that, since it's quite a complex one? If so, you can mark the PR as collaborative and work on it together.

Hi, this is looking like a good start, thank you! I wanted to suggest that you build on #1237, because it is almost the same! We'll want to make a function which runs each time there is an update to an image... it could even be each time there's a MouseUp, maybe? And it'll write a new "saved state" into localStorage, and similarly to #1237, not just the selected ones but all the images.

Shall we wait a little bit until #1237 is resolve then? Thanks!

Hi Jeff, I'm little bit confused how to get started on this. Could you please drop some hints? Thanks!

Hi Jeff, just a little hint, please

Hi @vanithaak -- so, I had realized that I was also working with @leilayesufu on a parallel track on parts of this, in #1161 -- and we had found that the problem was it only downloaded images that have been selected, so we needed to modify generateExportJson() to have a an onlySelected parameter defaulting to true, but so that we can override it if we want ALL images:

#1161 (comment)

Once this modification is made, we can use generateExportJson() for both downloading a JSON file, and storing the JSON file in localStorage. If you or @leilayesufu are interested in continuing to work on this, I think that's the key. It could even be done as its own PR -- and then we can begin using generateExportJson(false) to collect ALL images, for saving in localStorage OR for download. Does that make sense?

vanithaak · 2022-12-09T20:42:15Z

@jywarren I'd appreciate your input on this and a direction on where to begin If @vanithaak or @leilayesufu are still willing.

Hi @7malikk, thank you so much for asking. Please go ahead to work on this feature. Would love to see your work!

segun-codes · 2022-12-09T23:02:31Z

Concerning the first statement in this comment, I believe we should overwrite the last changes, as I am assuming if we are saving it, it would be in the localStorage and not locally? and localStorage as we've agreed that it is limited in space.

OK, let's start by overwriting. But we can note in a comment that it could be changed in the future (if we wanted, say, to create an "undo" feature).

The unique identifier is a very good idea and I agree we should not mix it up with this. But I will note it down for when we eventually get to that PR.

Agreed! Shall we open a new issue for it, perhaps?

@jywarren, you mirrored my thought on the consideration for supporting 'undo' operation. The 'undo' operation will only have to be limited to a lean number of save points given the memory size constraint on localStorage.

jywarren · 2022-12-10T16:20:41Z

@jywarren, you mirrored my thought on the consideration for supporting 'undo' operation. The 'undo' operation will only have to be limited to a lean number of save points given the memory size constraint on localStorage.

Agreed, we can think about it later, but I've for example used a rule like "overwrite if last save is within 5 minutes ago" and there can also be a hard rule like "limit total # of saves to 50 and delete older ones when adding a new one". Thanks!

segun-codes · 2022-12-10T18:29:56Z

@jywarren, you mirrored my thought on the consideration for supporting 'undo' operation. The 'undo' operation will only have to be limited to a lean number of save points given the memory size constraint on localStorage.

Agreed, we can think about it later, but I've for example used a rule like "overwrite if last save is within 5 minutes ago" and there can also be a hard rule like "limit total # of saves to 50 and delete older ones when adding a new one". Thanks!

Okay let's see.

7malikk · 2023-02-06T14:48:02Z

@jywarren per our conversation on saving a map to localStorage does the work @vanithaak accomplished set a good base?
If yes where do i insert the alert that reads “There are N saved maps… would you like to recover…?”

jywarren · 2023-02-06T20:57:17Z

Hi, I think we need to design some kind of new interface for this. Could we show a list of saved maps and, maybe, how many images each save has, when it was saved? And have a button on each to recover? Would you like to try to create a sketch of how this UI might look? Thank you!

7malikk · 2023-02-07T10:46:10Z

Hi, I think we need to design some kind of new interface for this. Could we show a list of saved maps and, maybe, how many images each save has, when it was saved? And have a button on each to recover? Would you like to try to create a sketch of how this UI might look? Thank you!

Sure, I would like to get that done @jywarren

7malikk · 2023-02-07T15:11:49Z

Hello @jywarren I was thinking we could use a modal designed like so:

What do you think?

jywarren · 2023-02-07T15:16:01Z

That looks great! It could be done with bootstrap components pretty easily right?

…

On Tue, Feb 7, 2023, 10:12 AM Malik ***@***.***> wrote: Hello @jywarren <https://github.com/jywarren> I was thinking we could use a modal designed like so: [image: Saved Maps] <https://user-images.githubusercontent.com/75104021/217283516-6af515ab-113d-4400-8f34-a3c459ee8f84.png> What do you think? — Reply to this email directly, view it on GitHub <#1225 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6JZT2TDYV4PLVNALMHLWWJQ37ANCNFSM6AAAAAARMLEVUM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

7malikk · 2023-02-07T15:22:58Z

That looks great! It could be done with bootstrap components pretty easily right?

Yea, i believe so

jywarren · 2023-02-07T17:12:02Z

To consider for where to place the button/link to open saved maps list modal!

jywarren · 2023-02-07T17:15:56Z

Here's the latest sidebar top UI. Are we over-complicating the "save" button or feature in introducing this new way of saving, and how could our design address a risk of "overcomplicating" to a new user?

7malikk · 2023-02-08T14:01:30Z

Here's the latest sidebar top UI. Are we over-complicating the "save" button or feature in introducing this new way of saving, and how could our design address a risk of "overcomplicating" to a new user?

I believe like you said, what we might see as giving the users options may look like complexities to the user.

But in the end I believe it'll be beneficial to the the user. They'd just have to get used to it?

What do you think? @jywarren

7malikk · 2023-02-08T14:02:49Z

To consider for where to place the button/link to open saved maps list modal!

This seems like the best spot, if we're deciding the modal does not come first

But we do have to decide on where the save to localStorage button will be if there will be one. @jywarren

7malikk · 2023-02-08T14:08:41Z

I believe like you said, what we might see as giving the users options may look like complexities to the user.

But in the end I believe it'll be beneficial to the the user. They'd just have to get used to it?

What do you think? @jywarren

Further more, if we do decide to add the save to LocalStorage, I was taking a look at the bootstrap dropdown I believe it'll suffice.
Where one action could be to download locally and the other to save to local storage. @jywarren

jywarren · 2023-02-08T16:27:07Z

Where one action could be to download locally and the other to save to local storage. @jywarren

That does sound like a good idea! Would the dropdown present both options each time, or would the main button have a default action?

For example, this button has other options, but the main one is "Squash and merge". What might be the default for our Save button?

I think it might be worth making a more visible button outline (using a different bootstrap style or color) as well to make it more clear, what do you think?

jywarren · 2023-02-08T16:28:12Z

As to complexity, having a strong default can really help with that. Then, new users will just use the default. Others can use the more advanced features hidden under the dropdown. How does that sound?

7malikk · 2023-02-08T18:23:42Z

@jywarren, my initial thought was that the button would trigger the dropdown which contains the two possible actions being: download or save to localStorage. With a descriptive tooltip, even new users won't find it complex in my opinion.

Using the squash and merge type, I believe only curious users would check what the dropdown does.

7malikk · 2023-02-08T20:27:37Z

Hello @jywarren What do you think about this?

jywarren · 2023-02-08T23:28:12Z

@jywarren, my initial thought was that the button would trigger the dropdown which contains the two possible actions being: download or save to localStorage. With a descriptive tooltip, even new users won't find it complex in my opinion.

This sounds great. I am not sure that's the way Bootstrap dropdowns normally work but they can probably be configured that way!

jywarren · 2023-02-08T23:30:43Z

Regarding the diagram, i like it! Very easy to read. The part about max # of maps is also a good thing to think ahead to. I think it would be a very long time before we hit the limit though... isn't localStorage like 4 mb or something?

7malikk · 2023-02-09T06:34:35Z

This sounds great. I am not sure that's the way Bootstrap dropdowns normally work but they can probably be configured that way!

I'd get on it and see

7malikk · 2023-02-09T06:57:28Z

Regarding the diagram, i like it! Very easy to read. The part about max # of maps is also a good thing to think ahead to. I think it would be a very long time before we hit the limit though... isn't localStorage like 4 mb or something?

That's true, 4-5 mb

7malikk · 2023-02-10T19:58:21Z

Hello, @jywarren Here is the current look of the save button,
What are your thoughts?
I am awaiting a merge on #1345 as a lot of changes there is core.

jywarren · 2023-02-10T19:59:14Z

looks perfect!!

jywarren · 2023-02-15T21:44:22Z

I know we're discussing in #1357, but also just let me know when this is ready to merge!

vanithaak and others added 16 commits October 17, 2022 00:52

detect key from longer URL

87f5076

Merge branch 'main' of https://github.com/vanithaak/Leaflet.Distortab…

f6225c3

…leImage

Merge branch 'publiclab:main' into main

4976617

Merge branch 'publiclab:main' into main

c71fc06

Merge branch 'publiclab:main' into main

8db3a8e

Merge branch 'publiclab:main' into main

1dca9e3

fixes

7cbcb80

Merge branch 'main' of https://github.com/vanithaak/Leaflet.Distortab…

24df784

…leImage

Merge branch 'publiclab:main' into main

47f1aa2

Merge branch 'publiclab:main' into main

07c7e94

Merge branch 'publiclab:main' into main

773c2fe

Merge branch 'publiclab:main' into main

13c6402

updates

67b2862

save map coordinates in local storage

8407d0f

error fixes

4a1339a

made a new method to save map in localStorage

a03c45c

vanithaak and others added 4 commits October 23, 2022 21:42

error fixes

e409e69

error fixes X2

415f0da

error fixes X3

44d3a4b

Merge branch 'main' into localStorage

f9e8212

jywarren mentioned this pull request Oct 25, 2022

save map coordinates in localStorage #1220

Closed

5 tasks

Merge branch 'main' into localStorage

eb09b9e

vanithaak and others added 4 commits October 25, 2022 23:01

removed overwriting of coordinates

57613e6

removed overwriting of coordinates

5f3e111

error fixes

a798be5

Merge branch 'main' into localStorage

96dc9f1

jywarren reviewed Oct 30, 2022

View reviewed changes

7malikk closed this Dec 10, 2022

7malikk reopened this Dec 10, 2022

jywarren mentioned this pull request Jan 17, 2023

Download json file of saved positions #1237

Merged

5 tasks

7malikk mentioned this pull request Feb 15, 2023

save map dropdown #1357

Merged

5 tasks

Uh oh!

Conversation

vanithaak commented Oct 23, 2022

Uh oh!

gitpod-io Bot commented Oct 23, 2022

Uh oh!

vanithaak commented Oct 23, 2022

Uh oh!

vanithaak commented Oct 25, 2022

Uh oh!

jywarren Oct 30, 2022

Choose a reason for hiding this comment

Uh oh!

jywarren Oct 30, 2022

Choose a reason for hiding this comment

Uh oh!

vanithaak Nov 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vanithaak Nov 14, 2022

Choose a reason for hiding this comment

Uh oh!

jywarren Nov 26, 2022

Choose a reason for hiding this comment

Uh oh!

vanithaak commented Dec 9, 2022

Uh oh!

segun-codes commented Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jywarren commented Dec 10, 2022

Uh oh!

segun-codes commented Dec 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

7malikk commented Feb 6, 2023

Uh oh!

jywarren commented Feb 6, 2023

Uh oh!

7malikk commented Feb 7, 2023

Uh oh!

7malikk commented Feb 7, 2023

Uh oh!

jywarren commented Feb 7, 2023 via email

Uh oh!

7malikk commented Feb 7, 2023

Uh oh!

jywarren commented Feb 7, 2023

Uh oh!

jywarren commented Feb 7, 2023

Uh oh!

7malikk commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

7malikk commented Feb 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

7malikk commented Feb 8, 2023

Uh oh!

jywarren commented Feb 8, 2023

Uh oh!

jywarren commented Feb 8, 2023

Uh oh!

7malikk commented Feb 8, 2023

Uh oh!

7malikk commented Feb 8, 2023

Uh oh!

jywarren commented Feb 8, 2023

Uh oh!

jywarren commented Feb 8, 2023

Uh oh!

7malikk commented Feb 9, 2023

Uh oh!

7malikk commented Feb 9, 2023

Uh oh!

7malikk commented Feb 10, 2023

Uh oh!

jywarren commented Feb 10, 2023

Uh oh!

vanithaak Nov 5, 2022 •

edited

Loading

segun-codes commented Dec 9, 2022 •

edited

Loading

segun-codes commented Dec 10, 2022 •

edited

Loading

7malikk commented Feb 8, 2023 •

edited

Loading

7malikk commented Feb 8, 2023 •

edited

Loading