Refactor slider state management

[vedhav]

Closes #321

Changes:

1. The toggle_slider_ui and toggle_slider_server can only be used to create dichotomous slider now. There was no instance of single value slider created. So, there is no need to create it and increase the complexity.
2. Removal of the keep_range_slider_updated in favor of keep_slider_state_updated to keep the states updated based on other widget inputs.
3. Updated the modules that uses this widget. Note that the tm_g_gh_lineplot does not use the keep_slider_state_updated and directly updates the state reactiveValues.

Check all the modules that use the toggle_slider module:

• tm_g_gh_boxplot
• tm_g_gh_correlationplot
• tm_g_gh_density_distribution_plot
• tm_g_gh_lineplot
• tm_g_gh_spaghettiplot
• tm_g_gh_scatterplot (deprecate in favor of tm_g_gh_correlationplot)

[gogonzo]

Three (or even four) states maintained in the same moment is are not needed.

• selected_state is actual selection from any (slider and numeric inputs)
• changes in slider update numeric_state
• changes in numeric update slider_state

Given above, each state needs to have same values which can be simplified by the single state shared by slider, numeric and also returned to "keep_slider_updated".

[vedhav]

Thank you @gogonzo I was able to remove an unwanted reactiveVal numeric_update_state. However, the other three variables initial_state, selected_state, and selected_state are needed.

Here are my comments related to the state variables:

1. The min, max, value, step can be changed by other widgets. When this is done, both the slider and numericInput should update their values. Hence the initial_state is needed.
   2 When the slider range is set by the numericInput, it can be done in two ways:
   i. The new range is bigger than the old range (old range was [0,10] and new range is [0,20])
   When this happens we have to increase the selected value of the slider & set the new ranges.
   ii. The new range falls between the older ranges (old range was [0.10] and new range is [4,8]
   When this happens we have to just set the new value of the sliders and the old ranges remain the same.
   So, it is important that we store the initial_state and changing the min and max from this variable will prevent us from achieving the slider state setting for the above user actions.
2. selected_state is the main state we maintain to return, which the module will use.
3. If we consider using a single state variable to manage the slider we create cyclic reactive dependency, because we have to render the slider when the state is updated. So, we have a variable slider_update_state which is set when initial state is changed or when the state is changed using the numericInput.

I think some comments can help here. I'll try to come up with better naming to make it easier to understand.

[gogonzo]

However, the other three variables initial_state, selected_state, and selected_state are needed.

All above can be simplified to:

• data_range (so called initial)
• range (current lower/upper boundary for slider)
• value (current selection)
• step

I think change counter is not even needed because technically keep_slider_state_updated can directly change above. SO the code can look like this

state <- list(data_min = reactiveVal(), ...) # list of reactiveVal to not trigger all observers

observeEvent(state$data_range(), {
  # update range if necessary
  # update values if outside the new range
})

observeEvent(state$value(), input$toggle, {  
  # update numeric if visible and if values differ from input$numeric
})

observeEvent(state$value(), state$range(), input$toggle, {  
  # update slider if visible and if values differ from input$slider
})

observeEvent(input$slider, {
  # update state$value
})

observeEvent(input$numeric, {
  # update state$value
  # update state$range if they went outside of the data range
})

return(state)

---

I'm pretty sure that selected_state and slider_state in the same time are not needed. I think a single state object, so that

numericInput and sliderInput are changing state and are changed when state changes.

[vedhav]

Thank you, let me try to implement this.
The change counter was needed to reset the sliders to the default state when there was no change in min/max/value/step made by the other widget changes but we needed to reset the slider states (this will fail when user changed the slider ranges and other widget does not reset this).

[gogonzo]

Beautiful. Issue fixed, test covered, -300 lines net in the R/ directory 💪

[m7pr]

Wooop wooop wooop!!!