Skip to content
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

Check if hierarchySeparator presents in the options object #1561

Merged
merged 2 commits into from Jul 31, 2017

Conversation

igor-dv
Copy link
Member

@igor-dv igor-dv commented Jul 31, 2017

Issue: When using options plugin and not providing the hierarchySeparator it's overridden by a NULL . In this case hierarchy will not created.

What I did

I've added a check for the hierarchySeparator prop is owned by an object.

How to test

TBD

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

I think this is pretty ugly/confusing code, but tested and it works, and want to get it deployed before more people use 3.2.x

@codecov
Copy link

codecov bot commented Jul 31, 2017

Codecov Report

Merging #1561 into master will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1561      +/-   ##
==========================================
+ Coverage   20.44%   20.45%   +0.01%     
==========================================
  Files         241      241              
  Lines        5248     5245       -3     
  Branches      649      647       -2     
==========================================
  Hits         1073     1073              
- Misses       3677     3679       +2     
+ Partials      498      493       -5
Impacted Files Coverage Δ
addons/options/src/preview/index.js 0% <0%> (ø) ⬆️
addons/info/src/components/Props.js 37.2% <0%> (ø) ⬆️
...ponents/left_panel/stories_tree/tree_decorators.js 30.88% <0%> (ø) ⬆️
lib/ui/src/modules/ui/libs/hierarchy.js 45.45% <0%> (ø) ⬆️
lib/ui/src/modules/api/configs/init_api.js 40.47% <0%> (ø) ⬆️
addons/info/src/components/Node.js 38.88% <0%> (ø) ⬆️
addons/knobs/src/components/types/Object.js 5.81% <0%> (ø) ⬆️
app/react/src/client/preview/error_display.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/types/Select.js 7.81% <0%> (ø) ⬆️
addons/info/src/components/markdown/code.js 24.13% <0%> (ø) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85de359...e6980d3. Read the comment docs.

@shilman shilman merged commit 3eef46e into master Jul 31, 2017
@shilman shilman deleted the fix-hierarchySeparator-default-null-in-options branch July 31, 2017 23:47
@usulpro
Copy link
Member

usulpro commented Aug 1, 2017

OMG! Thanks for fixing this! ❤️

@igor-dv
Copy link
Member Author

igor-dv commented Aug 1, 2017

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants