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

Add RegExp option for hierarchySeparator #1545

Merged
merged 4 commits into from
Jul 30, 2017
Merged

Add RegExp option for hierarchySeparator #1545

merged 4 commits into from
Jul 30, 2017

Conversation

usulpro
Copy link
Member

@usulpro usulpro commented Jul 30, 2017

Issue: #151

hierarchySeparator option allows only a string with RegExp

What I did

hierarchySeparator could take a Regular Expression directly (as well as strings)

How to test

run cra-kitchen-sink

@usulpro usulpro added addon: options cleanup Minor cleanup style change that won't show up in release changelog ui labels Jul 30, 2017
@usulpro usulpro added this to the v3.2.0 milestone Jul 30, 2017
@usulpro usulpro requested review from shilman and igor-dv July 30, 2017 17:32
@@ -7,14 +7,26 @@ export function init() {
// NOTE nothing to do here
}

function regExpStringify(exp) {
if (!exp) return null;
Copy link
Member

Choose a reason for hiding this comment

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

if(typeof exp === 'string') return exp;
if(Object.prototype.toString.call(exp) === '[object RegExp]') return exp.source;
return null;

@codecov
Copy link

codecov bot commented Jul 30, 2017

Codecov Report

Merging #1545 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1545      +/-   ##
==========================================
- Coverage   20.53%   20.52%   -0.01%     
==========================================
  Files         241      241              
  Lines        5220     5222       +2     
  Branches      644      641       -3     
==========================================
  Hits         1072     1072              
+ Misses       3660     3654       -6     
- Partials      488      496       +8
Impacted Files Coverage Δ
addons/options/src/preview/index.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/down_panel.js 23.52% <0%> (ø) ⬆️
lib/ui/src/libs/key_events.js 23.25% <0%> (ø) ⬆️
addons/info/src/components/markdown/code.js 24.13% <0%> (ø) ⬆️
app/react-native/src/preview/story_kind.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/left_panel.js 25% <0%> (ø) ⬆️
addons/knobs/src/components/types/Color.js 8% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_routing.js 28.94% <0%> (ø) ⬆️
...ponents/left_panel/stories_tree/tree_decorators.js 30.88% <0%> (ø) ⬆️
addons/knobs/src/react/WrapStory.js 12% <0%> (ø) ⬆️
... and 18 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 7ecadce...4cad0b0. Read the comment docs.

Copy link
Member

@igor-dv igor-dv left a comment

Choose a reason for hiding this comment

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

cc @joscha

@@ -266,7 +266,7 @@ storiesOf('component.Button', module)

// Atomic

storiesOf('Cells¯\\_(ツ)_/¯Molecules.Atoms/simple', module)
storiesOf('Cells/Molecules.Atoms/simple', module)
Copy link
Member

Choose a reason for hiding this comment

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

😢 ok..

const channel = addons.getChannel();
if (!channel) {
throw new Error(
'Failed to find addon channel. This may be due to https://github.com/storybooks/storybook/issues/1192.'
);
}
const options = {
...newOptions,
hierarchySeparator: regExpStringify(newOptions.hierarchySeparator),
Copy link
Member

Choose a reason for hiding this comment

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

what will happen if null will be passed here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

it will be ignored

@usulpro
Copy link
Member Author

usulpro commented Jul 30, 2017

so we can merge it if nobody objects?

@usulpro usulpro merged commit b02fec0 into master Jul 30, 2017
@usulpro usulpro deleted the regExpStringify branch July 30, 2017 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addon: options cleanup Minor cleanup style change that won't show up in release changelog ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants