(INT) app crash with RGB/diff coloring

[kaijagahm]

Overarching theory, since I can't find a pattern yet: this could be related to #24. Maybe the app is just overloading?

Documentation of when this occurs:

1. I had been showing Jim the map. We clicked around a bunch in points mode, then went to About and How to Use (don't remember which order). Finally went to interpolation mode. I had three sentences selected and we were testing out the RGB coloring. Then I clicked "reset all" in the left sidebar, and the app crashed. I can't replicate this crash.

[kaijagahm]

Aha! @ianneidel has figured out what's causing this. When you have three sentences selected in interpolation mode and you have the color set to RGB, adding another sentence and clicking apply crashes the app. That makes sense with my documented instance of the app crash above.

Similarly, when you have two sentences selected and you have the color set to "difference" (presumably either 1-2 or 2-1) and then you add another sentence (or, presumably, when you click reset EDIT: not when you click reset! Only when you add another sentence and click apply.), it crashes.

I think this has something to do with the tweaks that Jake Riley helped me implement, where the value of the color selector gets carried over if it's included in the new set of options. Will look into this.

[kaijagahm]

Okay, now I'm almost sure that this has to do with the thing Jake and I set up. I tested a hypothesis for whether the app would fail in points mode when I did a specific thing, and indeed, it failed as predicted.

Here's the deal: I have the following lines in the code for PTS

# (PTS) Update color criteria choices -------------------------------------------
  observeEvent(input$sentencesApply|input$sentencesReset, {
    val <- input$colorCriteriaPoints # this is the value that *was* selected before the update goes through
    choices1 <- c("Sentence 1 ratings", "Selected criteria") # these are the choices that are available for 1 sentence

    if(nSentences() == 1){
      updateSelectInput(session, "colorCriteriaPoints",
                        choices = c("Sentence 1 ratings", "Selected criteria"),
                        # set `selected` to the previously-selected value *only* if that value is contained in the available choices now.
                        selected = ifelse(val %in% choices1, val, "Sentence 1 ratings"))
    }else{
      updateSelectInput(session, "colorCriteriaPoints",
                        choices = c(paste0("Sentence ", activeSentences(), " ratings"),
                                    "Mean rating",
                                    "Median rating",
                                    "Min rating",
                                    "Max rating",
                                    "Selected criteria"),
                        selected = val)
    }
  }, 
  ignoreInit = T,
  label = "oeUpdateColorCriteriaPoints")

So what's going on there is this: previously, there was a problem with the app where whenever you changed the data (selected a new sentence, changed the ratings, etc.) and clicked "Apply", the app would lose your selection for input$colorCriteriaPoints. It did that because updating dat() would trigger updateSelectInput for the colorCriteriaPoints selector. It makes sense for it to trigger that because colorCriteriaPoints needs to listen to the number of sentences available. But a side effect of recreating the selectInput every time was that the previous selection got lost.

note: it's occurring to me now that I could maybe have set the selector to update when nSentences() or some other indicator changed, not just when dat() changed. But I think I considered and rejected that idea--don't remember why. At this point, it's probably easier to keep it as is.

So with the help of Jake Riley from the R4ds Slack, I added the above piece of code. The goal is to capture the current selected value in the selectInput, and then re-build the selectInput with new choices, and then if the old selected value is contained in the new menu of choices, have that be the 'selected' value. If not, just have 'sentence 1 ratings' be the selected value.

But the code above only half-implements this. I can't quite figure out why it's failing... I feel like it's on the tip of my tongue.

What I want to be able to do is set val to the old selected choice, and then also set a variable previousChoices that takes something like input$colorCriteriaPoints[choices]. Except I don't think that is possible: someone wanted to do something similar (extract the 'choices' argument from a selectInput object) in this SO post and it looks like they couldn't. Seems like the solution is to define the choices globally, and then have both the selectInput and this observer use that same global value.

So maybe instead of defining the choices dynamically in the code (which I do here by using paste() with activeSentences()), I need to define a separate reactive() outside of this observer which creates a vector of choices, and then feed that into this observer, and then also use that to check whether the old choice is contained in the new choice menu. Or something like that.

[kaijagahm]

I've been really terrible about documenting this issue: the above is a rambly train of thought. Let me try to do better.

• There was a bug in interpolation mode. When "Difference" (either 1-2 or 2-1) was selected in colorCriteriaInterpolation and then a sentence was added (bringing nSentencesI() up to 3 and eliminating the "Difference" options), the app would crash.
• Similarly, the app would crash when "rgb" was selected from colorCriteriaInterpolation and then a sentence was added (bringing the number of sentences up to 4 and eliminating the "rgb" option).
• Jonathan Trattner and I figured out why this was happening. I have several updateSelectInput statements, inside oeUpdateColorCriteriaInterpolation, that updated the possible choices in colorCriteriaInterpolation based on nSentencesI(). I had correctly specified logic within that statement to base the available colorCriteriaInterpolation choices on nSentencesI() so that when 2 sentences were active, "Difference" options would appear, but not when a different number were active (and the same for rgb). I had also written logic that would maintain the previous selection in colorCriteriaInterpolation IF that was possible. So for example, if you had "Sentence 2 ratings" selected with 2 active sentences, and then you added a third sentence, "Sentence 2 ratings" would stay selected because it is a valid option for both the 2-sentence state and the 3-sentence state.
• The above-described logic was supposed to revert back to "Sentence 1 ratings" if the previously-selected choice was no longer available after changing the number of sentences. So, the desired behavior would be this: if nSentencesI() == 2 and you've selected one of the "Difference" options, and then you switch to 3 sentences, where "Difference" is no longer available, the selection will revert to "Sentence 1 ratings".
• The bug occurred due to a problem with the updateSelectInput statements contained in oeUpdateColorCriteriaInterpolation. The documentation says this:

The input updater functions send a message to the client, telling it to change the settings of an input object. The messages are collected and sent after all the observers (including outputs) have finished running.

• This means that even though the colorCriteriaInterpolation was appropriately updating, eventually, it was doing so only after all of the other observers in the app had finished running. This created a problem, since there was an observer, oeUpdateColorColI, that depended on having an up-to-date value in colorCriteriaInterpolation in order to plot the hexes properly in oeUpdateHexColorsData.
• I dug into it and didn't find any way to override this run-last behavior for updateSelectInput().

As of 20 January 2021, Ian has written a workaround. The observer that relied on the updated value of colorCriteriaInterpolation was oeUpdateColorColI, which updated a reactiveVal called colorColI().

Ian added some logic into oeUpdateHexColorsData. The code does the following: 1) create a new object within the observer called colorcolilocal, equal to colorColI(). 2) check whether there's a mismatch between colorcolilocal and nSentencesI 3) if so, reassign colorcolilocal to what it's supposed to be 4) replace colorColI() with colorcolilocal in the rest of the oeUpdateHexColorsData observer so that the hex colors update based on the correct value.

This seems to work, as far as I can tell, so I've gone ahead and made the change in this commit: https://github.com/kaijagahm/ygdpDashboard/commit/dce5d01b0d8e4922768d1cbbd39bf099dba26445

[kaijagahm]

I thought I remembered that there was a parallel bug in PTS mode, but now I can't find it. I added some print statements to aid in debugging: https://github.com/kaijagahm/ygdpDashboard/commit/4a2c6fd0ca5afb004dac9a6ab50e6d679b4bc448, but so far the app seems to run fine in PTS mode.

Going to close this issue for now and deploy the app. Will open a new issue if I discover a bug in PTS mode.