-
Notifications
You must be signed in to change notification settings - Fork 0
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
add ability to lock axes #185
Conversation
Codecov ReportPatch coverage is 📢 Thoughts on this report? Let us know!. |
@@ -8,6 +8,14 @@ | |||
<input type="checkbox" class="form-check-input" style="vertical-align:bottom;" v-model="logScaleYAxis"> | |||
</div> | |||
</div> | |||
<div id="lock-axes" class="row my-2"> | |||
<div class="col-5"> | |||
<label class="col-form-label">Lock axes</label> |
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.
<label class="col-form-label">Lock axes</label> | |
<label class="col-form-label">Lock y axis range</label> |
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.
as discussed, this actually locks both axes (for example when someone zooms in on the graph), if i dont lock both then the x axis autoscales when youre zoomed in which gives the graph a stretch that wasn't there before!
@@ -8,6 +8,14 @@ | |||
<input type="checkbox" class="form-check-input" style="vertical-align:bottom;" v-model="logScaleYAxis"> | |||
</div> | |||
</div> | |||
<div id="lock-axes" class="row my-2"> |
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.
An exciting thing happens if you interact with the log scale toggle too.
- Start with your example showing exponential growth
- Lock the y axis
- Select "log scale"
Now the y axis will go to something like 10^61800 and the lines are all along the bottom.
It's not the end of the world - unlocking the axis returns to sensible values, and then you can relock the axis and it behaves well in log mode. But this does not tolerate well moving between those modes
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.
fixed!
app/static/src/app/plot.ts
Outdated
@@ -19,7 +19,8 @@ export const margin = { | |||
}; | |||
|
|||
export const config = { | |||
responsive: true | |||
responsive: true, | |||
displayModeBar: true |
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.
has this PR changed the plotly config to remove the floating bar (despite this saying true)? People use that to download plots so it's important to retain. I see it's there on the version on wodin-dev still at least
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.
ahh good point, it was setting it as false because i thought it would have controls for zoom, allowing users to bypass the zoom lock when axes are locked, however plotly is smart and if i lock the axes then the zoom options dont show up! so it is added back in
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.
thanks, this is fab
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.
LGTM. Bit confused about where we got to with the zoomability. I do still seem to be able to zoom with axes locked, but then it snaps back to the locked range on re-run.
newPlot(el as HTMLElement, baseData.value, layout, config); | ||
const configCopy = { ...config } as Partial<Config>; | ||
|
||
if (lockYAxis.value && !toggleLogScale) { |
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.
So this means it always does autorange if we're updating because of logScale value change?
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.
Yes so the log scale completely changes the axis, if you lock the axis as before there are very weird things y axis values, so instead we don't lock y axis when they toggle between the log and normal scale
Ahh sorry forgot to update the comment, in the first implementation, they could not zoom in but instead keep the viewport fixed! But this is not what we wanted so now they can zoom in as before but it locks the y axis on re run essentially |
This lets users lock the axes to give a better comparison of how the graph changes between runs. Users can simply toggle the lock axes checkbox in graph settings and this disables zoom on the graph, locks the axes so that any subsequent updates have the same view range and domain.
To test try out this code in wodin:
If you keep re-running it you can see the y axis changes a lot.
To remember the last axes ranges we store them in state. You can also zoom in when you haven't locked axes and then lock them in a smaller view! On lock or unlock of the axes the graph redraws with autorange as before.