Sage-Future
Add auto-sum-to-100 button to multi choice questions
Add a button that sums exclusive multi-choice questions to 100. It should have the following behaviour:
- If all options have prediction values, they should be scaled so their sum total is 100 while maintaining the current ratio between them
- If all-but-one option has a prediction value, the value for the remaining option should be 100 - the sum of all other options
- If more than one option doesn't have a prediction value, any remaining probability mass should be split evenly across all un-filled options
Start by looking at components/predict-form/question-types/MultiChoiceQuestion.tsx
Hey Jonny – if you're not in a rush I reckon this is a good issue for a first time contributor. Let me know if you've got any time pressure here but if not I'd like to take this. 😄
Hey - no time pressure on my end, so you're welcome to go for it! Let me know if you need a hand with anything and I'll find some time.
Awesome, thanks :). Just wanted to get started a bit today (won't have much time until Sunday), let me know how this looks! (loom) in terms of design. I'll look to add a few tests before passing it over for review.
I'm rounding to 1 decimal for the moment, felt that was a good place to start.
Yo @jonnyspicer – got some time on this, code mostly clean but I have some issues I'm running into. Could you help me with these two questions?
Behavior issue
Behavior is still the same as in the last loom with regards to happy path but this requirement:
If more than one option doesn't have a prediction value, any remaining probability mass should be split evenly across all un-filled options
Logically leads to this issue:
- 60% for option 1, 60% for option 2, XX (undefined) for option 3
- Press Sum to 100
- Option 3 gets set to 0, total still over 100
Thoughts? I'd suggest this change to the logic (bolded):
- If all options have prediction values, they should be scaled so their sum total is 100 while maintaining the current ratio between them
- If not all options have prediction values and the sum is over 100%, scale while treating un-filled options as 0
- If not all options have prediction values and the sum is under 100%, split the remainder between un-filled options
With this behavior in my hypothetical example we'd end up with 50%, 50% 0%
Jest issue
Jest seems to not want to run my tests – I see the following error:
Cannot use import statement outside a module
I'm sure it's just some config settings that need changed but I timed out today and wanted to bring it to your attention before I get scope creep and go off and try to fix it – I don't want charge ahead with changes to the config if you've set it up as it's set up now for a specific reason.
Let me know if you're happy with me tackling the jest issues in this PR or if you can quickly spot that I'm doing something silly – I'd appreciate it! 😄
Next steps
I'm going to be busy over Xmas so in no rush for a response. I'm not blocked though – without getting a response I'll try to get a fix for Jest and will implement my solution for the behavioral issue, worst comes to worst it'll be a little rework.
@adam-binks once again if I'm tagging to many folk just let me know – I notice you've been active recently and thought you might have an opinion on the best behavior here in case Jonny is off on hols
Hey @slowSort sorry for the late reply - I was indeed off on holiday :)
On the auto-sum logic, I think the other option is to display the existing "options must sum to < 100" message. I like this as
- It makes the logic simpler
- Users won't have their percentages scaled down when pressing the button - I think this makes for a better UX to keep the behaviour simpler too.
What do you think?
On the Jest setup issue, I think it's because Jest isn't currently configured to use ESModules, which we use throughout the project, so it's unable to compile the code to run the tests against. I'm happy for you to update this, or I can do it if that'd be easier. I'm back working again so should hopefully be more responsive!
Hey @jonnyspicer, thanks for the reply!
I like that auto-sum logic, It's less magic for the user to have to figure out and it's fairly intuitive behavior. Don't want someone having to think through what pressing a button just did. For now I'll focus on getting it working with a loom video of me manually testing.
On the Jest setup – I've given it a go for an hour or two and ran into a few landmines along the way. I've put it down for now, I'd appreciate if you could take a look! Perhaps I've missed something simple in the setup...
If you do take a look, might be worth reading this (https://jenchan.biz/blog/dissecting-the-hell-jest-setup-esm-typescript-setup) as it was basically my experience trying to get tests up and running. If Fatebook is not 100% settled on a test runner and you also get bogged down in setting up ESM in Jest it might be worth considering how hard it would be to move over to Vitest as an alternative
Yeah I remember having to do something like allowlisting all the ESM I wanted to use in the config, it was pretty grim. Pivoting to Vitest before Jest is particularly entrenched in our codebase is a great idea, I appreciate it. We're not spending much of our bandwidth on Fatebook atm but next time I'm working on it then I'll see how easy it'd be to swap over.