search

ArgoCanada / argoFloats

Tools for analyzing collections of oceanographic Argo floats
https://argocanada.github.io/argoFloats/index.html
17 stars 7 forks source link

Add undo function for mapApp() #517

Closed j-harbin closed 3 years ago

j-harbin commented 3 years ago

Dan has started this in the branch named undo. My plan is to finish https://github.com/ArgoCanada/argoFloats/issues/516 and then focus on this.

dankelley commented 3 years ago

My checklist is as follows, with work in the "undo" branch.

  1. [x] Fix merge conflicts that arose with the recent changes to "develop". Done in 37e59b70f86d67969522cc42861d0a365c7b3e23
  2. [x] Simplify my code. I don't think we need to push to the state stack with every change of state, only with every redraw. That means I can trim a few dozen lines, clarifying the action a lot. Done in 37e59b70f86d67969522cc42861d0a365c7b3e23
  3. [ ] Get it working on float ID inserted by double-click.
  4. [x] Think about incorporating the settings. This will require saving the colours, line widths, etc in state, which means observing the changes (which, I think, we don't do now). Done in commit fa42833b99df718b6b84bba17be6e5f3e6a763cd of the undo branch

I won't rush on this because, as J and I have agreed, our CRAN-submission window has been moved by a week. (Basically, I think if we make any nontrivial changes in a week, then the window needs to shift from the Friday at the end of that week to the Friday at the end of the following week.)

dankelley commented 3 years ago

This is coming along. @j-harbin has colour-state working. I am fiddling with ID-state enacted by double-click, which was requiring 3 undo operations to undo. This is because it was drawing the map not once, but 3 times:

  1. present time, with the chosen ID
  2. whole time of that chosen ID
  3. the same as 2

and so an Undo was required for each. I had not noticed this before but now I can see it flashing 3 times. It might be a bit slower than before, and therefore visible, because we are temporarily printing quite a lot of information on the undo stack. Anyway, the refreshing is very annoying, and I want to get rid of it.

I thought the problem was that we are saving state and updating the UI. I'm not too sure we need to do the latter, at least not first for observing dblclick, then again for observing input$ID. I commented-out the UI updating and the app still seems to work fine, but my theory that this is the cause of redrawing must be wrong, because it still draws 3 times.

I don't think I am going to see the problem today, so I'm pushing "undo" commit 410ad59adf0da2c552d64bd7e2a0a67de6a2398d and will return to this tomorrow.

j-harbin commented 3 years ago

Closed during ftf with Dan.