Skip to content

Multi axes #77

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

Merged
merged 29 commits into from
Nov 17, 2017
Merged

Multi axes #77

merged 29 commits into from
Nov 17, 2017

Conversation

bpostlethwaite
Copy link
Member

Annotation of important pieces of this code will arrive in a day or so

This requires plotly.Axes methods so we shamefully hack those on for now
When autorange=true range components are not visible
TraceSelector was not updated when we moved to the container system.
Tests!
A variables main file defines the customizable variables in the
editor. This is only partially complete. Better standardization and
coverage is required. Use a default vars file to assign plotly
defaults to the customizable variables.
The underlying modal is useful to other dropdown like components so
that has been split into something called ModalBox
<LayoutPanel group="Style" name={_('Axes')}>
<AxesFold name={_('Titles')}>
<AxesSelector />
<Dropdown
Copy link
Member Author

Choose a reason for hiding this comment

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

This is what the API for axes will look like. A layout connected component wrapping an Axes connected component wrapping a generic container connected component.
If you took away the components and left only the higher order wrappers it would like this:

connectLayoutToPlot(connectAxesToLayout(connectToContainer(<Field />)));

In fact in principal that should work. I better make sure it does and test it.

value = defaultValue;
} else {
value = propValue;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

the new multiAxes framework has resulted in vastly simpler widget code. They don't need to know about multiValued anything so far!

* Test that we can connectLayoutToPlot(connectAxesToLayout(Panel))
*/
function setMultiValuedContainer(intoObj, fromObj, key, searchArrays) {
var intoVal = intoObj[key],
Copy link
Member Author

Choose a reason for hiding this comment

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

@alexcjohnson this is your mergeAttr function from graph_edit.js :)

Copy link
Member Author

Choose a reason for hiding this comment

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

This file contains the meat of handling multi-values. It's pretty similar to WS1

if (fv === MULTI_VALUED) {
return MULTI_VALUED_PLACEHOLDER;
}
return fv;
Copy link
Member Author

Choose a reason for hiding this comment

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

unpackPlotProps, the last thing to run before the widget layer handles the transformation of a MULTI_VALUED value (which contains an escape character) to MULTI_VALUED_PLACEHOLDER which is ---.
Special purpose components can and do override this particular behaviour when necessary.

export function noShame(opts) {
if (opts.plotly && !opts.plotly.Axes) {
opts.plotly.Axes = {
list,
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Leave it for now - in fixing plotly/plotly.js#2125 I'm adding fullLayout._subplots which will also list x and y axes, which will moot this, mostly at least - might still need special handling for 3D and such but suffice to say it's in flux right now.

@bpostlethwaite
Copy link
Member Author

@alexcjohnson this is ready to look at. I have annotated the more interesting parts of the code. Thanks!

if (isNumeric(defaultValue)) {
valueUpdate = defaultValue;
} else {
// TODO smarter handling depending if user decrements or increments?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose you could make some argument like increment should bump all values up to the biggest one and decrement should bump all values to the smallest... but that sounds like a major headache and I'm not sure if it's really worth much.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Looks great! I made a small comment in slack about simplifying props vs context in AxesSelector, you can address that here or later. Other than that 💃 !

@bpostlethwaite bpostlethwaite merged commit 697ee3d into develop Nov 17, 2017
@bpostlethwaite bpostlethwaite deleted the multiAxes branch November 17, 2017 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants