Skip to content

Commit

Permalink
FIX addon-contexts: edge case bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
Leo Y. Li committed Apr 24, 2019
1 parent 06e057e commit d66aa30
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 6 deletions.
2 changes: 1 addition & 1 deletion addons/contexts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ be shown at first in the toolbar menu in your Storybook.
rule of thumbs, whenever collisions made possible, always the first wins.
5. Query parameters are supported for pre-selecting contexts param, which comes handy for visual regression testing.
You can do this by appending `&contexts=[name of contexts]=[name of param]` in the URL under iframe mode. Use `,`
to separate multiple contexts (e.g. `&contexts=Theme=Forests,Language=Fr`.
to separate multiple contexts (e.g. `&contexts=Theme=Forests,Language=Fr`).

## 📖 License

Expand Down
4 changes: 2 additions & 2 deletions addons/contexts/src/manager/components/ToolbarControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ export const ToolbarControl: ToolbarControl = ({
const [expanded, setExpanded] = React.useState(false);
const paramNames = params.map(({ name }) => name);
const activeName =
// validate the selected name
(paramNames.concat(OPT_OUT).includes(selected) && selected) ||
// validate the integrity of the selected name
(paramNames.concat(options.cancelable && OPT_OUT).includes(selected) && selected) ||
// fallback to default
(params.find(param => !!param.default) || { name: null }).name ||
// fallback to the first
Expand Down
3 changes: 2 additions & 1 deletion addons/contexts/src/preview/libs/getContextNodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ type _getMergedSettings = (
) => ContextNode;

export const _getMergedSettings: _getMergedSettings = (topLevel, storyLevel) => ({
nodeId: topLevel.title || storyLevel.title || '',
// strip out special characters reserved for serializing
nodeId: (topLevel.title || storyLevel.title || '').replace(/[,+]/g, ''),
icon: topLevel.icon || storyLevel.icon || '',
title: topLevel.title || storyLevel.title || '',
components: topLevel.components || storyLevel.components || [],
Expand Down
2 changes: 1 addition & 1 deletion addons/contexts/src/preview/libs/getRendererFrom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('Test on aggregation of a single context', () => {

it('should skip wrapping when props is marked as "OPT_OUT"', () => {
const testedProps = OPT_OUT;
const testedOption = {};
const testedOption = { cancelable: true };
spiedAggregator([fakeTag, fakeComponent], testedProps, testedOption)();
expect(h).toBeCalledTimes(0);
});
Expand Down
2 changes: 1 addition & 1 deletion addons/contexts/src/preview/libs/getRendererFrom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const _getAggregatedWrap: _getAggregatedWrap = h => (
// when set to disable
options.disable ||
// when opt-out context
props === OPT_OUT;
(options.cancelable && props === OPT_OUT);

return isSkipped
? vNode
Expand Down

0 comments on commit d66aa30

Please sign in to comment.