-
Notifications
You must be signed in to change notification settings - Fork 9
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
MI-59 exclude all studies #197
base: dev
Are you sure you want to change the base?
Conversation
You could have kept the first PR, because the branch to be merged into can be changed after creation. (In fact, for a while I didn't realise there was any other way of doing it!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is fine, but there is a problem which I assume is from MI-28. If you check the boxes too quickly you go into an infinite loop, where they keep checking and unchecking.
Nooooo! I thought I'd fixed that! Can you give me a set of precise reproducible instructions please? |
I don't think I can. I can reliably reproduce it using the default continuous dataset, but not always at the same point. Sometimes it starts immediately and sometimes not. The only thing I can say with certainty is that if you click them one at a time and watch the table on tab 1a, if you wait for the table to update before the next click then the problem doesn't occur. Good luck! |
@tommorris168 I've reverted the disconnected network issue on another branch. Are you happy for this to be merged now? |
It's crashing when I remove all the studies. |
All studies can now be excluded without crashing the app