Skip to content

Expand values into arrays as needed when setting with nestedProperty #1581

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

Closed
wants to merge 1 commit into from

Conversation

rreusser
Copy link
Contributor

@rreusser rreusser commented Apr 11, 2017

See #1580

This is just speculative. I don't necessarily think it's a good idea, but I'm submitting it to confirm whether it's actually a bad idea. If the current value is not an array or an object and you try to set an array value, it wraps the current value in an array before setting. Unless I'm mistaken, this should only affect cases that currently throw an error.

@rreusser rreusser changed the title Expand values into arrays when setting with nestedProperty Expand values into arrays as needed when setting with nestedProperty Apr 11, 2017
@rreusser
Copy link
Contributor Author

rreusser commented Apr 11, 2017

The best options I can come up with:

  1. this PR, except maybe fill in all values up to the requested index with the current value.
  2. restyle/relayout/nestedproperty need more knowledge about the schema so they can enable this only for arrayOk attributes
  3. we should disallow non-data attributes that fall into this pattern (setting data attributes with an accessor like this isn't an interesting use case anyway, but nestedProperty results in modal Plotly.restyle #1580 highlights why the difficulty is problematic for non-data attrs that need to get set in the ui)
  4. work around the problems by digging into fullData in client code

@rreusser
Copy link
Contributor Author

Closing this as a discussion point but probably not the right solution.

@rreusser rreusser closed this Apr 11, 2017
@etpinard etpinard deleted the nested-property-expand-array-setters branch January 14, 2019 15:40
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.

None yet

1 participant