Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

43 Marvellous Mitigator Pick-n-Mix Selection box 🍬 #98

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

craig-parylo
Copy link
Contributor

closes #43

Alternative approach to selecting mitigators

This PR offers an alternative approach to selecting mitigators by replacing the carefully curated list of mitigators with the freedom to make your own.

image



image

Benefits:

🍫 Freedom to create your own list of mitigators,
🍬 Finding mitigators is made easy by using the interrelated filters and search boxes,
🍭 Mitigators can be quickly un-selected by clicking the 'x' in each tag.

Known issues:

  • The filters rely on functions from the GitHub version of the {datamods} package. These functions have been hard-copied in as the CRAN version resulted in issues with {bslib}, specifically it removed the ability for cards() to fill the screen. The GitHub version does not do this, so presumably this will eventually make it to the CRAN version too.
  • Hard-copying the functions from the {datamods} package adds quite a bit of import calls to the NAMESPACE file.
  • I've read (from forums) the use of the {datamods} functions may interfere with the Shiny bookmarks. TO BE TESTED.
  • This implementation requires the user to select mitigators before they see the app in action. This adds a 'burden' to the UX over the current approach, though this could possibly be fixed by having default mitigator selections?

To do:

  • Test the mitigator selections using the bookmark feature on the Posit.connect instance.
  • Explore what mitigators would be good to have pre-selected by default.

…lection approach. These are hard-coded as the version of {datamods} from CRAN interferes with bslib components - specifically the cards lose their ability fill the page.
…ion to filter and pick mitigators, an action button to add the picked mitigators to the list of selected mitigators and a 'selectizeInput()` object to list the selected mitigators with the option to click to remove any unwanted ones.
…gators()` takes the mitigator_lookup and prepares it for use in the app, `add_to_selected_mitigators()` adds any newly-selected mitigators to the selected mitigator list.
…or selections ui and comments-out code that drove the previoius mitigator selection process and adds an shiny::observeEvent triggered by the new action button to add mitigators to the selected list.
… {datamods} to drive the select_group_ui/server
@craig-parylo craig-parylo self-assigned this Dec 2, 2024
@matt-dray
Copy link
Contributor

Thank you Craig, as ever! You did a live demo earlier today and we chatted about it. Two features we noted:

  1. A 'clear' button might be helpful for removing all the currently-selected mitigators.
  2. Incorporating the mitigator codes into the mitigator name strings (which appear in the 'selected mitigator' box but not the 'mitigator' dropdown) and that we may want the code to go after the name (rather than before, which is what we have in the 'selected mitigator' box). This will allow for searching by the code in the 'mitigator' dropdown, but won't obscure the first few words of the full mitigator name.

We also discussed:

  • the benefit of the user being faced with the 'select at least one mitigator' when they arrive at the app, which forces a mindful interaction and a choice, which might be preferable to choosing default mitigators to start the app with
  • proceeding with {datamods} as-is, but keeping an eye on its development (this can become a techdebt-labelled issue after this PR gets merged)

As mentioned, I'm going to try deploying this branch now to see if the bookmarking works as intended in a production environment. I think that needs to be confirmed before merging, and before committing anything related to (1) and (2) above.

@matt-dray
Copy link
Contributor

matt-dray commented Dec 5, 2024

Ah, I'm a fool, Shiny server bookmarking works locally (a shiny_bookmarks/ folder is created in the project root when you click the bookmark button in the locally-run app). I gave it a go by selecting a subset of schemes and mitigators and adjusting some pointrange plot settings. Unfortunately, this results in the app spitting back_all_ mitigators and schemes when the bookmark URL is used. Have you got a link to the where you read about the {datamods} bookmark interference issue? Might have a read around and we can discuss next week.

(In doing this I also spotted a small bookmarks-related issue: it's expected behaviour in retrospect, but the bookmark will return all sites if the 'all sites' checkbox was checked, even if the user removed sites from the 'sites to visualise' box before clicking the bookmarks button. I'll record that separately.)

(I also typed 'borkmarks' accidentally when writing this, which seems apt!)

@craig-parylo
Copy link
Contributor Author

Thank you very much for deploying and testing out the bookmarks feature. I'm relieved it is mostly working as expected - and I had no idea the bookmarking works locally too!
Here's where I first read about a possible issue between {datamod}'s select_group functions and bookmarking. I'll get cracking on the tasks we discussed yesterday. Thank you again.

- changes to on-the-fly calculation of mixture distributions,
- updates 'add mitigator' button label to include a count of mitigators that are in-scope to be added to the selected list,
- adds observeEvent handle clearing selected mitigators on button click,
- increases default height for heatmap plots to accomodate new schemes with longer names
- adds space between 'reset filters' and 'add to selection' button,
- changes 'add to selection' button to use shiny library with a light colouring as more compatible with server-side updates to button label,
- adds action link to clear the list of selected mitigators.
…tor_code}]') for lookup list to allow for searching for mitigators by code and ensure compatibility between available mitigator and selected mitigator lists.
@craig-parylo
Copy link
Contributor Author

Additions

These additional commits include:

  • a button to quickly clear all selected mitigators
  • mitigators listed by their name AND code, which allows for searching by code and visual confirmation of selected mitigators,
  • 'add to selected' button now includes a count of mitigators in-scope.

image

Additional benefits include:

  • mixture distributions are calculated on-the-fly which allows for a more responsive UX. This change is made possible by the mitigator selection UI encouraging mindful selections of (usually) fewer mitigators so the calculations can be achieved in a reasonable time,
  • heatmaps have more vertical height to display the longer names for the newly-added schemes when x-axis is rotated 90 degrees.

@ghobro
Copy link
Collaborator

ghobro commented Dec 19, 2024

@Fikriyudin11 - just a bookmark of the mitigator selection functionality I mentioned yesterday that might be a good idea to review in the new year if we go down the route of making an app for comparing the mitigator impacts under scenarios. Craig has provided very illustrative figures at the top and commentary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reconsider the approach to select mitigators
3 participants