Skip to content

Add failling test for coerce#1125

Closed
rreusser wants to merge 1 commit into
masterfrom
pause-button-fix
Closed

Add failling test for coerce#1125
rreusser wants to merge 1 commit into
masterfrom
pause-button-fix

Conversation

@rreusser

@rreusser rreusser commented Nov 9, 2016

Copy link
Copy Markdown
Contributor

This PR adds a failing test for Lib.coerce. It's pretty short:

var attrs = { 
  x: {
    valType: 'info_array',
    freeLength: true,
    items: [
      { valType: 'any' },
      { valType: 'any' },
      { valType: 'any' }
    ]   
  }
};
out = coerce({x: [[], 10]}, {}, attrs, 'x');
expect(out).toEqual([[], 10]);

It fails because the output is equal to [undefined, 10] instead of [[], 10].

The error goes away if I remove the third valType: 'any', suggesting that freeLength doesn't quite do what it sounds like it does. freeLength has no effect either way, but I've included it since I'd expect it to help. It seems it's refusing to set empty arrays, but I'm not sure why.

@rreusser rreusser added status: in progress bug something broken labels Nov 9, 2016
@rreusser

rreusser commented Nov 9, 2016

Copy link
Copy Markdown
Contributor Author

Found the source of the problem: https://github.com/plotly/plotly.js/blob/master/src/lib/nested_property.js#L243

The solution is not straightforward. The result is that you can't actually pass an empty array through plotly. That makes a pause button a bit awkward.

@rreusser

rreusser commented Nov 9, 2016

Copy link
Copy Markdown
Contributor Author

Solution: The cause of this is that Lib.nestedProperty is written to aggressively clean up after itself. This touches a lot of things so is best not modified. For a pause button where an affirmative empty array [] is meaningful and does not indicate the item should be deleted, it seems the best solution, contingent upon #1121, is to require [null] for pausing instead of [].

@rreusser rreusser closed this Nov 9, 2016
@etpinard
etpinard deleted the pause-button-fix branch November 15, 2016 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant