Error should be raised when we try to set slider of output variable [#19] #35
Conversation
| margin-top: 5px; | ||
| border: 1px solid #000; | ||
| font-size: 1em; | ||
| outline: none !important; |
There was a problem hiding this comment.
What was the reason behind this (outline)?
There was a problem hiding this comment.
this should eliminate the blue outline that appears on focus to follow the same styling as other gifsmos inputs, e.g. .Burst-input and .Burst-button screenshots related:
src/actions/index.js
Outdated
| dispatch(updateBurstSliders(sliders)); | ||
| }; | ||
|
|
||
| const updateBurstSliders = sliders => ({ |
There was a problem hiding this comment.
In the rest of this file, the actions appear to be at the top. Is there a particular reason you've placed this one apart from the rest?
There was a problem hiding this comment.
good catch! moved from thunk section to action section
|
@japamat -- am i using this wrong or are there bugs in this PR? I tried pasting in the following: https://www.desmos.com/calculator/gzmoe8w5hx there are 3 sliders in this graph. But when I try to select one of them from the dropdown, i either get a warning message about not defining a slider (for the second two), or a straight up error (for the first one). can you reproduce? |
There was a problem hiding this comment.
@japamat - nothing in here looks problematic, but i think there are some 🐛 in here. happy to chat more when you're free.
src/components/Burst.js
Outdated
| /> | ||
| > | ||
| <option value={null} defaultValue> | ||
| {burstSliders.length ? 'Choose Slider' : 'No Sliders'} |
There was a problem hiding this comment.
can you change the 'Choose Slider' text to something shorter? the text is getting cut off:
There was a problem hiding this comment.
Changed to Pick Slider, now it fits
src/components/Burst.js
Outdated
| {burstSliders.map(exp => { | ||
| return ( | ||
| <option key={`slider-${exp.id}`} value={exp.id}> | ||
| {' '} |
|
@mmmaaatttttt |
[issue #19]
the problem:
When the user inputs an expression index that doesn't correspond to a slider, the expression can be overridden by the slider, screenshots & gif below for y=mx + b:
capture burst, slider index set to 1, produces the following:
My aim was to only allow the user to select from sliders that wont override expressions by filtering the expressions list from calculator.getExpressions() for only expressions whose latex follows the pattern of 1 variable being assigned a numerical value. I implemented a dropdown menu, with default values that throw errors when the users tries to capture an image burst. If there are no sliders, the defaul option is simply No Sliders.
To do this, I needed to add
burstSliderto the redux store in ui, which gets updated every time the burst panel is opened.@ctlusto
@alissarenz
@mmmaaatttttt
Chris, please wait for Alissa to approve before review!