Let numeric input be set manually

[asbates]

Let's a numeric input be set via range slider or input boxes.

• Only one type, slider or input box, is available at a time.
• Which is visible is controlled by a checkbox.
• When switching input types, the new input gets set to the values of the old input.

Closes #133

[gogonzo]

When I ran cdisc app state didnt seem to change instantly. I changed the values and waited for the result and If I didn't focus on the summary I wouldn't have a clue that it didn't change the state yet. I see the point of the change and I think that it should be automatically switched back to slider when changes made - which suggests dropdown menu.

Maybe unrealistic vision, but what about tooltip menu popping up when cursor is over slider elements "irs-from" and "irs-to". I think it's possible to make a js on this element as it has a specific class.

[nikolas-burkoff]

What happens if you choose values manually which are between two steps of the slider or beyond the edges of the slider e.g. if you select [0.0000001, 0.0000002] - if you switch to the slider and then back again is everything intuitive? Similarly if you close the card and reopen it etc.

I'd also add validation here (what happens if you select say "e" (which is allowed in a numeric input, not sure if it's allowed in a range input) or don't enter one/both of the values - does it behave sensibly?

Also if changing the value by the api (e.g. to a value not selectable in the slider) does everything behave as expected? It might be worth using the example RangeFilterState app in the roxygen as a good place to test things

[BLAZEWIM]

So it is not intuitive that you have to click the box to enter values, and then unclick to apply changes. But generally I prefer this input box over window popping up, cause it is simplier (is it thou?) but does the trick.

[chlebowa]

Seems to be working nicely now. One thing I don't like is the Enter range manually checkbox, I would rather it be a button labeled Switch to manual and Switch to slider or similar.

Nik's comment on validation is a must now as anything not coercible to numbers will break the app. The question is: do we want to add shinyvalidate to the filter panel? My first instinct is no but I have no strong arguments.

Out of range values and values between slider points are handled sufficiently well now, I believe. The one thing that could be corrected is this:
When selecting a value out of range, the selection is corrected to the appropriate limit of the possible range, e.g. if data allows 4.3-7.9 and one selects 0-10, the selection is adjusted to 4.3-7.9. This is reflected in both the slider and the summary but NOT in the state of the numericInputs.

[BLAZEWIM]

(...) The question is: do we want to add shinyvalidate to the filter panel? My first instinct is no but I have no strong arguments.

It is already there, near the date / datetime input.

[chlebowa]

(...) The question is: do we want to add shinyvalidate to the filter panel? My first instinct is no but I have no strong arguments.

It is already there, near the date / datetime input.

It's not in Imports, though.
All right, shinyvalidate it is.

[gogonzo]

(...) The question is: do we want to add shinyvalidate to the filter panel? My first instinct is no but I have no strong arguments.

We should avoid shinyvalidate in the filter panel - we should never return validate error from the FilteredData. What we do instead we cast_and_validate values to the proper range/choices and display notification

teal.slice/R/FilterStateDate.R

Line 357 in 17a8fc6

showNotification(

[chlebowa]

(...) The question is: do we want to add shinyvalidate to the filter panel? My first instinct is no but I have no strong arguments.

We should avoid shinyvalidate in the filter panel - we should never return validate error from the FilteredData. What we do instead we cast_and_validate values to the proper range/choices and display notification

teal.slice/R/FilterStateDate.R

Line 357 in 17a8fc6

showNotification(

So FilterStateRange$cast_and_validate should be updated here?

[BLAZEWIM]

We should avoid shinyvalidate in the filter panel - we should never return validate error from the FilteredData. What we do instead we cast_and_validate values to the proper range/choices and display notification

Ok, now I see it being removed, but why exactly we should not return validate error from the FilteredData?

[gogonzo]

Ok, now I see it being removed, but why exactly we should not return validate error from the FilteredData?

When everything is set properly by the app developer it would be silly if filter-panel return errors when poor user would just change something in the way we didn't expected to do. Validate errors are wrong as every shiny process after filter-panel (teal and modules) should handle the case when data return error. Validate is fine within teal_module where we (or anyone using teal) can directly control inputs/outputs.

So FilterStateRange$cast_and_validate should be updated here?

It can stay - if it occurs it means it's our fold that we allowed user to select something wrong.
If app developer used api and used NA or Inf in selected then it's app developer error (specified selected in way not descibed in docs of the set_selected)- so it should throw error also.

[chlebowa]

So FilterStateRange$cast_and_validate should be updated here?

It can stay - if it occurs it means it's our fold that we allowed user to select something wrong.
If app developer used api and used NA or Inf in selected then it's app developer error (specified selected in way not descibed in docs of the set_selected)- so it should throw error also.

You mean we should allow the user to crash the app with a single accidental keystroke? Once $cast_and_validate raises that error, the app is dead.

[gogonzo]

So FilterStateRange$cast_and_validate should be updated here?

It can stay - if it occurs it means it's our fold that we allowed user to select something wrong.
If app developer used api and used NA or Inf in selected then it's app developer error (specified selected in way not descibed in docs of the set_selected)- so it should throw error also.

You mean we should allow the user to crash the app with a single accidental keystroke? Once $cast_and_validate raises that error, the app is dead.

What I mean is exactly opposite, user shouldn't be able to break the app accidentally or intentionally.
What accidental keystroke can break the app?
In the app user isn't able to set inputs so that cast_and_validate throw. If it's possible then it's a bug which we should fix.

[chlebowa]

So FilterStateRange$cast_and_validate should be updated here?

It can stay - if it occurs it means it's our fold that we allowed user to select something wrong.
If app developer used api and used NA or Inf in selected then it's app developer error (specified selected in way not descibed in docs of the set_selected)- so it should throw error also.

You mean we should allow the user to crash the app with a single accidental keystroke? Once $cast_and_validate raises that error, the app is dead.

What I mean is exactly opposite, user shouldn't be able to break the app accidentally or intentionally. What accidental keystroke can break the app? In the app user isn't able to set inputs so that cast_and_validate throw. If it's possible then it's a bug which we should fix.

One can accidentally type "r" instead of "5" and cast_and_validate will raise an error because the input will return NA of type logical.

[asbates]

I've made some updates.

Instead of a checkbox, we now have a switch with a label. Not especially happy with the label text but it's what I could think of.

I've also made some updates in terms of error handling/validation for manual inputs. In each of these cases, we show a notification and reset the input to the current state of the class.

• Can't set either lower or upper bound to 'e'. (allowed by browser, NA in Shiny server)
• Can't specify lower bound > upper bound
• Can't specify number that isn't an option in the slider.

The last one is probably the most controversial. I could see an argument for not doing this but I think we should. Otherwise we could have the slider set to one value and the manual inputs to another. In that case, what is the state of the class? I also view the slider as the main input so the manual input should be driven by that. Remember the slider has been there all along and the manual input is just an extra convenience. Another case is if my range is 0 to 5 say, and I set 0 to 0.00000001. If we allow that and don't show the notification, then $set_selected() actually changes this to 0 and again the input is out of sync with the class state which is misleading. I also think that doing different things for the manual inputs vs. the slider is out of scope for this issue. I think if we want different behavior than current, we should scope that out and be specific before implementation.

[lcd2yyz]

Thanks @asbates for the updates!
I also think switch is better than checkbox for this. But one minor things - the color of the switch different from the slider, can we possibly have a consistent coloring for all our UI elements based on bootstrap themes?

• Can't specify number that isn't an option in the slider.

I do have some concerns about this item though. What do you mean exactly by "option in the slider"? Does this mean I can only choose values marked by the tick marks (eg. -3.1, -2.5 etc)? Or do you just mean the entered number has to be somewhere within the slider range?

One of the main reason to enable the manual range is to provide users with more precise control on the data they are filtering. For example, some key numbers for lab tests may have very specific thresholds and we need to enable the user to provide the exact cutoff (eg. normal ferritin level in female is 11-307, we cannot achieve precise filtering if the tick marks are at 300 and 320).

I also think that doing different things for the manual inputs vs. the slider is out of scope for this issue. I think if we want different behavior than current, we should scope that out and be specific before implementation.

Agree! Opened a new issue here #231

[chlebowa]

(It seems I forgot to sent his last night.)

Instead of a checkbox, we now have a switch with a label. Not especially happy with the label text but it's what I could think of.

How about having a button for each version instead of one switch?

• Can't specify number that isn't an option in the slider.

The last one is probably the most controversial. I could see an argument for not doing this but I think we should. Otherwise we could have the slider set to one value and the manual inputs to another. In that case, what is the state of the class? I also view the slider as the main input so the manual input should be driven by that. Remember the slider has been there all along and the manual input is just an extra convenience. Another case is if my range is 0 to 5 say, and I set 0 to 0.00000001. If we allow that and don't show the notification, then $set_selected() actually changes this to 0 and again the input is out of sync with the class state which is misleading. I also think that doing different things for the manual inputs vs. the slider is out of scope for this issue. I think if we want different behavior than current, we should scope that out and be specific before implementation.

This is tricky. Since the slider ticks are not actual data points but prettified values, there is a mechanism whereby programmatic selection is corrected reset to the closest tick, so that the requested value is included in the subsetting expression. See ?contain_interval. I assume the same mechanism kicks in when you input values through a numeric input.

I would be satisfied with private$selected being set to what is currently visible in the input.

[nikolas-burkoff]

Yes if you can't set a specific value using the range input then it's not really worth bothering with the range input therefore I think #231 is a high priority.

We could even remove the slider input and just show the histogram (maybe with vertical lines showing the range selected) and the range input which would simplify things a lot...

I assume my environment isn't set up properly as when I run the example app cdisc app #165 I get an error:

Warning: Error in eventReactiveValueFunc: attempt to apply non-function
  103: active_datanames
  101: renderUI
  100: func
   87: renderFunc
   86: output$teal-main_ui-filter_panel-overview-table
    5: runApp
    4: eval [~/NEST/app.R#96]

so haven't been able to test what happens if you try to enter a numeric value that doesn't match the slider stepsize - if it just gives an error - or changes the input value to make a stepsize without giving a notification as to why - then although this could be merged into the refactor branch I don't think it should be merged into main until #231 is done

[chlebowa]

I assume my environment isn't set up properly as when I run the example app cdisc app #165 I get an error:

You may have to install teal from filter_panel_refactor@main.

[nikolas-burkoff]

Yup my environment wasn't happy - this looks good but if being released, I would, in the notification, imply that although right now the numeric value must match the slider values this will change soon as otherwise I suspect we'll have some unhappy users...

[asbates]

@lcd2yyz For the color of the switch, I set it to be the "info" color as this made the most sense to me. This will also allow it to match whatever theme is set. I can change it to "primary" if you like. This would again make it change depending on the theme.

I do want to note that for the slider color, it's histogram, radio button graphs, and other things, we aren't actually using the bootstrap theme. A lot of these items are hard-coded. So if the theme is different, these items will remain blue, and likely won't match the theme. Also I'm not 100% sure but I think the default Bootstrap colors are slightly different for different versions. In this case the colors won't match even if it's just a version change vs. a theme change.

For the "option in the slider" I mean we set the manual numbers to numbers available in the slider. This is what I mean by having different behaviors for different inputs (slider vs. box). I can see wanting the numeric to stay the same but I think this is getting into details that should be in another issue. I may have missed it but this wasn't discussed in the issue this PR adresses.

[asbates]

This is tricky. Since the slider ticks are not actual data points but prettified values, there is a mechanism whereby programmatic selection is corrected reset to the closest tick, so that the requested value is included in the subsetting expression. See ?contain_interval. I assume the same mechanism kicks in when you input values through a numeric input.

Yes exactly, the method $set_selected() adjust the values. So removing the check and notification if the manual input doesn't match slider ticks, the actual state may be different than what the user input.

So maybe we remove the out_of_range check but add a notification informing the user when the values they input will not be the values in set in $set_selected? If out_of_range block is removed, $set_selected may change the values the user input. I think they should be aware of that and we should update the input to reflect the actual state.

I would be satisfied with private$selected being set to what is currently visible in the input.

Which input? The slider or the box? Do we handle this differently for the slider and the box? Wouldn't this be confusing for the user. I'm not sure if you're saying this, but we shouldn't assign to private$selected directly (except initialization) because that's the point of having self$set_selected

[asbates]

We could even remove the slider input and just show the histogram (maybe with vertical lines showing the range selected) and the range input which would simplify things a lot...

It would indeed. It sounds to me like we are wanting different behavior for different inputs. I don't think that is a good idea. Or at a minimum we clearly communicate this to the user.

If we drop the slider, then we won't have to change the values in $set_selected which I think is a good thing. The histogram may be tricky but we could probably work it out.

[donyunardi]

The numericRangeInput provides the same step in the slider input, so clicking the up and down arrow in the input box will increase and decrease the value based on the step.

If a user enters a value that's outside of the range or not within the step, it will give a showNotification message and revert the value back to the last valid value according to the step.

One more route that I think we can explore is to replace numericRangeInput with selectInput, which means we will have two selectInput. The choices of these selectInputs will be the value of the steps.
However, the issue remains, user won't be able to freely enter any value but at least can see all the possibly choices to pick from.

If we want to give a total freedom for user to enter a range, then I like what @nikolas-burkoff suggested:

We could even remove the slider input and just show the histogram (maybe with vertical lines showing the range selected) and the range input which would simplify things a lot.

This would make things a lot simpler and we would only have one input to handle numeric ranges.

@lcd2yyz
Can we get your thoughts on this?

[lcd2yyz]

The freedom to specify any numerical value for the manual input range is very important for this feature to be valuable - we definitely need to enable this. We need to move away from the "steps" restriction when switching to manual input, so selectInput doesn't make sense to me at all.

I think providing the slider along with the manual input option will be a nice touch, because most users are familiar and accustomed to this type of UI for numerical ranges, and it's simply visually appealing. So I would strongly prefer to have both if possible.

Maybe I don't have the full grasp of all the check/validation that goes on behind the scene.... but could we just use a pair of numericInput from shiny-base?

[nikolas-burkoff]

The numericRangeInput and numericInput both have the up and down arrows to change the step, but both can be overwritten by users typing whatever numbers they want (including scientific notation).

Maybe I don't have the full grasp of all the check/validation that goes on behind the scene.... but could we just use a pair of numericInput from shiny-base?

So yes we can (though a range input is nicer) - the issue is with users jumping between slider and range input (and the fact that module developers (and app developers in teal::init) can make a call to the filter panel to set the range to what they want programmatically) where the former insists on values exactly in stepsize.

My proposal would be to get rid of the slider entirely and update the histogram to show the selected range in some way - like this (would want to add x axis labels to the histogram)

And if you really wanted to keep the slider-like behaviour in some way then you could add buttons to each side and each click would change the values in the range by a chosen step (but there would be NO snapping to a specific value, i.e. if "step" was 0.1 and 0.10004 was entered in the lower range and then the button clicked it would jump down to 0.00004

A lot of this may be out of scope for this issue, but thought it worth getting my thoughts down here for discussion

[chlebowa]

My proposal would be to get rid of the slider entirely and update the histogram to show the selected range in some way - like this (would want to add x axis labels to the histogram)

This sounds lightweight and easy to comprehend, so 👍. I would only add axis labels for the selected values (redundant, though).

And if you really wanted to keep the slider-like behaviour in some way then you could add buttons to each side and each click would change the values in the range by a chosen step (but there would be NO snapping to a specific value, i.e. if "step" was 0.1 and 0.10004 was entered in the lower range and then the button clicked it would jump down to 0.00004

The buttons on the sides I don't like so much. Too many clicks required to move the limit a significant distance and the user would have to change input mode. Also, a bit redundant with buttons in numeric inputs.

[gogonzo]

@nikolas-burkoff @chlebowa I fully support this idea. We can figure out range-shift later. At this moment I'm totally convinced with removal of the slider

@lcd2yyz @donyunardi what do you think about the above proposal?

[asbates]

Keep in mind if we remove the slider, then we should adjust our set_selected method. Because right now it alters the values it's given (sometimes). If we need the precision of numeric inputs, then we need to adjust set_selected. Otherwise it's misleading to users.

[asbates]

I still stand by statement this is getting out of scope though. I suggest we either merge this PR and fix with another issue. Or ditch the PR altogether and make the issue clear with the intended behavior.

[lcd2yyz]

Thanks all for thinking hard about potential solutions and coming up with the proposals! Sorry for what will be a long response, but hopefully it can add some new perspective to the problem at hand.

@donyunardi and I had a long discussion and tested a few things this afternoon. I still feel quite strongly of having both slider and manual range input together.
One example use case why having the slider is important is when scientists using the slider to explore some threshold/cutoff - when they want to split the population by a baseline lab value into high vs low, or into tertiles. They can easily visually ballpark the slider to mid-point or 2/3 of the range quickly, look at the statistics and decides if they want to adjust (and repeat the process). I would have loved @nikolas-burkoff's proposal if the red brackets are draggable. But if the brackets are simply visual markers, it may take quite a few guesses (if entering numbers) or clicking on the left/right arrow many times, in order to find the midpoint.

I understand the biggest challenge here is when user switches back and forth between the slider and numeric input. So below is an outline of what I would consider to be the ideal behavior when user interacts with this UI.

1. Upon app loading, slider is shown with the values set at default
2. User can drag the slider freely between the pre-defined (or default) range, slider will move in "steps" and anchors to closest step, user will only be able to modify the ranges by the "step". eg. steps are 5, 10, 15, 20, 25 etc, user can only drag the slider to value of c(10, 20) using the slider, teal.slice updates and applies selected range value to c(10, 20).
3. But what user really wants is using 17 - 21 as range, so he/she toggles to numeric range input and enters the number. teal.slice updates and applies the selected range value to c(17, 21).
4. For whatever reason, user toggles back to the slider UI (clicks on the "Enter manually" toggle, but does not do anything else). At this point, the selected range on the slider should continue to show c(17,21), and does not jump to the next closest "step", and teal.slice does NOT update the selected range values. This is very important!!
5. After checking the statistics, user want to explore more cutoffs, so starts to drag the slider, and the slider again moves by steps, and selects a new range of c(5, 25). teal.slice updates and applies the range value of c(5, 25).
6. User toggles back to the numeric range input, and the two boxes will show 5 and 25 respectively.

Another behavior @donyunardi and I discussed was user entering out-of-range values (compared to the range of available data) in the numeric input. For example, the observed min-max of the data is c(14, 35), but the user specified c(10, 40) via the input UI, or app developer specified that range as default via API. The slider UI should update to take the min/max between the data and the numerical inputs to use as the lower/upper bounds of the slider. This means the slider range should be reactive to both the filtered data and the numerical input.
An use case for this behavior is when using normal lab reference ranges to filter data. For example, the normal ref range for a lab test is c(10, 40), but the current data has an observed min-max of c(14, 35). The user/app developer may want to set a filter using the normal ref range, especially when added to reporter, because it's scientifically meaningful. Then as new data come in, where there exist abnormal lab values, it will be automatically filtered out from downstream analysis.

Lastly, I will leave the decision to merge or ditch this PR to Dony and the team, depends on whether some already-implemented functionalities can be repurposed for the desired UI behavior.

[nikolas-burkoff]

At this point, the selected range on the slider should continue to show c(17,21), and does not jump to the next closest "step",

I don't believe this is possible I'm afraid

[chlebowa]

4. For whatever reason, user toggles back to the slider UI (clicks on the "Enter manually" toggle, but does not do anything else). At this point, the selected range on the slider should continue to show c(17,21), and does not jump to the next closest "step", and teal.slice does NOT update the selected range values. This is very important!!

This may well be impossible. Slider ticks are calculated when the FilterState is initialized, based on the range of the data. Moreover, the way the slider is specified is one passes a minimum, a maximum, and a step, so the distances between ticks are identical. The step is computed so that there are always 100 steps. Adding another tick would mean updating the slider with a new step value, such that a tick would fall on the new, arbitrary value. Then the very number of ticks could change wildly, depending on this one arbitrary value. This is something we considered in a previous PR and decided to stay away. Doing that for two numbers (min/max) would be much more problematic.

5. After checking the statistics, user want to explore more cutoffs, so starts to drag the slider, and the slider again moves by steps, and selects a new range of c(5, 25). teal.slice updates and applies the range value of c(5, 25).

This would be quite difficult, even if the above were possible.

Another behavior @donyunardi and I discussed was user entering out-of-range values (compared to the range of available data) in the numeric input. For example, the observed min-max of the data is c(14, 35), but the user specified c(10, 40) via the input UI, or app developer specified that range as default via API. The slider UI should update to take the min/max between the data and the numerical inputs to use as the lower/upper bounds of the slider.

I don't like this, and for several reasons.

• In FilterState private$choices stores the available input values. For RangeFilterState, this is the range of the data upon instantiation. All input values, whether they come from the UI or the API, are validated against this range and adjusted to that range. This behavior cannot be realistically waived for a slider input, as one cannot drag a slider past its range, so allowing if for a different input source would be inconsistent.
• I don't think the app user should have the power to change input choices/ranges, this should be up to the app developer, if at all allowed. Implement a new API #187 will allow the app dev to set choices (limited with reference to the data) and that is to be validated against the data range. I suppose we could change that without too much trouble.

This means the slider range should be reactive to both the filtered data and the numerical input. An use case for this behavior is when using normal lab reference ranges to filter data. For example, the normal ref range for a lab test is c(10, 40), but the current data has an observed min-max of c(14, 35). The user/app developer may want to set a filter using the normal ref range, especially when added to reporter, because it's scientifically meaningful. Then as new data come in, where there exist abnormal lab values, it will be automatically filtered out from downstream analysis.

This begs the question: when a range like that is set and it is scientifically meaningful, should that fact be included in the subsetting call for reporting purposes, whether or not any data is dropped out? I think we should discuss this as a separate issue, preferably with some end user input.

[nikolas-burkoff]

I would have loved @nikolas-burkoff's proposal if the red brackets are draggable

Using plotly this is possible:

The black lines here are draggable (sadly they are draggable vertically as well but I would hope that could be configured?)

(code below)

Separately the example here (after "After @ firmo23's edit:") is really nice

library(shiny)
library(plotly)

data <- data.frame(x = runif(1000))

ui <- fluidPage(
  plotlyOutput("plot"),
  verbatimTextOutput("result")
)

server <- function(input, output, session) {
  output$plot <- renderPlotly(
    plot_ly(data, x = ~x,type="histogram") %>%
      layout(shapes = list(
        list(type = "line", x0 = 0.4, x1 = 0.4, 
             y0 = 0, y1 = 1, yref = "paper"),
        list(type = "line", x0 = 0.8, x1 = 0.8, 
             y0 = 0, y1 = 1, yref = "paper"))) |>
      # allow to edit plot by dragging lines
      config(edits = list(shapePosition = TRUE))  
  )
  
  selected_range_rv <- reactiveVal(c(0.4, 0.8))
  
  observeEvent(plotly::event_data("plotly_relayout"), {
    x <- plotly::event_data("plotly_relayout")
    if ("shapes[0].x0" %in% names(x)){
      selected_range_rv(c(x[["shapes[0].x0"]], selected_range_rv()[2]))  
    }
    if ("shapes[1].x0" %in% names(x)){
      selected_range_rv(c(selected_range_rv()[1], x[["shapes[1].x1"]]))  
    }
  })

  output$result <- renderText({
    sprintf("The selected range is from %f to %f", min(selected_range_rv()), max(selected_range_rv()))
  })
    
}

shinyApp(ui, server)

[chlebowa]

I would have loved @nikolas-burkoff's proposal if the red brackets are draggable

Using plotly this is possible:

Oh, I assumed plotly is off the table, but If not - this is excellent, especially the built-in slider shown in the SO example:

library(plotly)

x <- c(1:100)
random_y <- rnorm(100, mean = 0)
data <- data.frame(x, random_y)

fig <- plot_ly(data, x = ~x, y = ~random_y, type = 'scatter', mode = 'lines') %>%
  layout(xaxis = list(rangeslider = list()))

fig

[gogonzo]

@donyunardi none of the solutions propositions us all. Discussion exceeds the issue requirements and disagreement make it hard to merge. Since @lcd2yyz wish to keep slider I propose to:

1. Make a second attempt on the slider and ticks to find out if we can find robust solution - Issue would be to provide ticks to slider which can cover all possible data points (I'm sceptical if we can make it perfect and efficient in the same time).
2. Research and present implementation of Nik's proposal of interactive histogram.
3. If (1) is impossible we need a decision if we want to merge this PR or use (2)
4. Accept the problems mentioned by developers and merge PR with values snapping to the nearest possible.

For 1, 2 we need a separate issue

---

P.S. I wonder how sliders are done in other applications (spotfire, tableau).

[donyunardi]

I agree with @gogonzo proposal.
Let's hold on this PR and I'll open separate issues as suggested so we research, plan, and execute this properly.

[asbates]

Update: we had a discussion today and decided to keep this functionality as is. We will present it to users and get their feedback. Then make adjustments if needed.

I'm going to close this PR and open a new one based on the current filter panel refactor branch.