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

Disable autocommit and manually commit filters in study view #4502

Merged
merged 2 commits into from
Apr 6, 2023

Conversation

BasLee
Copy link

@BasLee BasLee commented Feb 2, 2023

Add a checkbox to the study page to disable 'auto submitting' of filters; and add a submit button that allows users to submit filters manually. (Netlify preview)

image
'Autosubmitting' of filters is enabled by default. It can be activated with the 'Manually submit' option, available in a new settings menu

Problem

When a user adds a new filter on the study page, it is automatically added and processed: data is requested from the backend and the panes are re-rendered. When studies grow and datasets get larger, study view panes become less responsive, making it quite laborious to add a few filters.

Solution

This PR adds a checkbox and a submit button. The checkbox allows users to disable 'autosubmitting'. New filters are now queued. Once the new submit button is clicked, queued filters are submitted all at once, only triggering a single rerender.

Disabling autocommit

image

When disabling autosubmit, filters are added to the queue. Panes are only updated when the submit button is clicked.

Visualizing submitted and queued filters

image

When disabling autosubmit, the filter bar color codes 1) newly added filters; 2) already applied filters; and 3) filters that are queued for deletion.

TODO

  • Move toggle to gear menu
  • Show which filters are submitted and which are not
  • Add e2e test

@BasLee
Copy link
Author

BasLee commented Feb 2, 2023

summary of feedback during refactorers stand-up:

  • move auto submit toggle to new gear button
    • more study view options can be moved there later on
  • move submit button closer to filters, before 'clear all filters' button
  • visualize which filters are applied and which are not yet submitted
    • add new line with queued filters (show submit button on this line?)

@cBioPortal cBioPortal deleted a comment from pvannierop Feb 21, 2023
@BasLee BasLee force-pushed the studyview-autosubmit-toggle branch 2 times, most recently from fa85a45 to c87289b Compare February 28, 2023 12:25
@BasLee BasLee requested a review from alisman February 28, 2023 12:49
* TODO: A better solution would be a uniform list of filters
* but this would require some refactoring.
*/
getBrowserWindow().hesitantPillStore = {} as PillStore;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not put these in the StudyView context?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, is "hesitant" a typo?

Copy link
Author

@BasLee BasLee Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 60e31e2
Pill stores are now in study view store, and study view store is passed to pills using the context

@alisman
Copy link
Collaborator

alisman commented Mar 1, 2023

@pvannierop @BasLee i did some testing here. I think there are probably some more tweaks that we would want to make to the UI before going live with this for our own users.

  • The "pending" style is not obvious enough
  • The hesitate mode state should be persisted?
  • I think the submit button should not show until you made a new filter and have something to submit.
  • The product team may want to weigh in on the placement of the submit button
  • Right now when you toggle the hesitate mode, some of the charts re-render, which is odd. We should see if we can
    prevent that.

I think a feature like this should be manually tested thoroughly by someone on your team who is not a developer.

It should also be well covered by e2e tests.

@BasLee
Copy link
Author

BasLee commented Mar 7, 2023

@alisman #4502 (comment)
Added local e2e tests in 6e33cd4

@BasLee
Copy link
Author

BasLee commented Mar 13, 2023

image

☝️Feedback from community call on March 7

Relevant changes:

@BasLee BasLee force-pushed the studyview-autosubmit-toggle branch from 0a337c5 to 9b5b740 Compare March 21, 2023 10:20
@alisman alisman force-pushed the studyview-autosubmit-toggle branch from d5f1923 to b1205a3 Compare April 6, 2023 18:32
@alisman alisman merged commit 13ad1a0 into cBioPortal:master Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants