Skip to content

Commit

Permalink
UI: Fix settings page route (about, shortcuts) (#7241)
Browse files Browse the repository at this point in the history
Fix settings page route(about, shortcuts)
  • Loading branch information
shilman authored Jul 10, 2019
2 parents fd780ec + c6402a7 commit 6b577eb
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 11 deletions.
10 changes: 7 additions & 3 deletions lib/api/src/modules/stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ type Direction = -1 | 1;
type StoryId = string;
type ParameterName = string;

type ViewMode = 'story' | 'info' | undefined;
type ViewMode = 'story' | 'info' | 'settings' | undefined;

export interface SubState {
storiesHash: StoriesHash;
Expand Down Expand Up @@ -262,7 +262,7 @@ Did you create a path that uses the separator char accidentally, such as 'Vue <d

// Now create storiesHash by reordering the above by group
const storiesHash: StoriesHash = Object.values(storiesHashOutOfOrder).reduce(addItem, {});

const settingsPageList = ['about', 'shortcuts'];
const { storyId, viewMode } = store.getState();

if (storyId && storyId.match(/--\*$/)) {
Expand All @@ -279,7 +279,11 @@ Did you create a path that uses the separator char accidentally, such as 'Vue <d
// we pick the first leaf and navigate
const firstLeaf = Object.values(storiesHash).find((s: Story | Group) => !s.children);

if (viewMode && firstLeaf) {
if (viewMode === 'settings' && settingsPageList.includes(storyId)) {
navigate(`/${viewMode}/${storyId}`);
} else if (viewMode === 'settings' && !settingsPageList.includes(storyId)) {
navigate(`/story/${firstLeaf.id}`);
} else if (viewMode && firstLeaf) {
navigate(`/${viewMode}/${firstLeaf.id}`);
}
}
Expand Down
34 changes: 34 additions & 0 deletions lib/api/src/tests/stories.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,40 @@ describe('stories API', () => {
setStories(storiesHash);
expect(navigate).not.toHaveBeenCalled();
});

it('navigates to the settings page if a existing page was selected', () => {
const navigate = jest.fn();
const store = {
getState: () => ({ viewMode: 'settings', storyId: 'about' }),
setState: jest.fn(),
};

const {
api: { setStories },
} = initStories({ store, navigate });

setStories(storiesHash);
expect(navigate).toHaveBeenCalledWith('/settings/about');
});

it(
'navigates to the first story in the store when viewMode is settings but' +
'non-existent page was selected',
() => {
const navigate = jest.fn();
const store = {
getState: () => ({ viewMode: 'settings', storyId: 'random' }),
setState: jest.fn(),
};

const {
api: { setStories },
} = initStories({ store, navigate });

setStories(storiesHash);
expect(navigate).toHaveBeenCalledWith('/story/a--1');
}
);
});

describe('jumpToStory', () => {
Expand Down
3 changes: 1 addition & 2 deletions lib/router/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ interface SeparatorOptions {
groupSeparator: string | RegExp;
}

export const knownNonViewModesRegex = /(settings)/;
const splitPathRegex = /\/([^/]+)\/([^/]+)?/;

// Remove punctuation https://gist.github.com/davidjrice/9d2af51100e41c6c4b4a
Expand Down Expand Up @@ -47,7 +46,7 @@ export const parsePath: (path?: string) => StoryData = memoize(1000)(

if (path) {
const [, viewMode, storyId] = path.match(splitPathRegex) || [undefined, undefined, undefined];
if (viewMode && !viewMode.match(knownNonViewModesRegex)) {
if (viewMode) {
Object.assign(result, {
viewMode,
storyId,
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const App = React.memo(({ viewMode, layout, size: { width, height } }) => {
);
});
App.propTypes = {
viewMode: PropTypes.oneOf(['story', 'info', 'docs']),
viewMode: PropTypes.oneOf(['story', 'info', 'docs', 'settings']),
layout: PropTypes.shape({}).isRequired,
size: PropTypes.shape({
width: PropTypes.number,
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/src/components/layout/container.js
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ Layout.propTypes = {
panelPosition: PropTypes.string.isRequired,
isToolshown: PropTypes.string.isRequired,
}).isRequired,
viewMode: PropTypes.oneOf(['story', 'info', 'docs']),
viewMode: PropTypes.oneOf(['story', 'info', 'docs', 'settings']),
theme: PropTypes.shape({ layoutMargin: PropTypes.number }).isRequired,
};
Layout.defaultProps = {
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/src/components/layout/desktop.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Desktop.propTypes = {
panelPosition: PropTypes.string.isRequired,
isToolshown: PropTypes.bool.isRequired,
}).isRequired,
viewMode: PropTypes.oneOf(['story', 'info', 'docs']),
viewMode: PropTypes.oneOf(['story', 'info', 'docs', 'settings']),
};
Desktop.defaultProps = {
viewMode: undefined,
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/src/components/layout/mobile.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ Mobile.propTypes = {
render: PropTypes.func.isRequired,
})
).isRequired,
viewMode: PropTypes.oneOf(['story', 'info', 'docs']),
viewMode: PropTypes.oneOf(['story', 'info', 'docs', 'settings']),
options: PropTypes.shape({
initialActive: PropTypes.number,
isToolshown: PropTypes.bool,
Expand Down
2 changes: 1 addition & 1 deletion lib/ui/src/components/preview/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ Preview.propTypes = {
}).isRequired,
storyId: PropTypes.string,
path: PropTypes.string,
viewMode: PropTypes.oneOf(['story', 'info', 'docs']),
viewMode: PropTypes.oneOf(['story', 'info', 'docs', 'settings']),
location: PropTypes.shape({}).isRequired,
getElements: PropTypes.func.isRequired,
queryParams: PropTypes.shape({}).isRequired,
Expand Down
6 changes: 5 additions & 1 deletion lib/ui/src/components/sidebar/SidebarStories.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,18 @@ const PlainLink = styled.a(plain);

const Wrapper = styled.div({});

const refinedViewMode = (viewMode: string) => {
return viewMode === 'settings' ? 'story' : viewMode;
};

export const Link = ({ id, prefix, name, children, isLeaf, onClick, onKeyUp }) =>
isLeaf ? (
<Location>
{({ viewMode }) => (
<PlainRouterLink
title={name}
id={prefix + id}
to={`/${viewMode || 'story'}/${id}`}
to={`/${refinedViewMode(viewMode) || 'story'}/${id}`}
onKeyUp={onKeyUp}
onClick={onClick}
>
Expand Down

1 comment on commit 6b577eb

@vercel
Copy link

@vercel vercel bot commented on 6b577eb Jul 10, 2019

Choose a reason for hiding this comment

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

Please sign in to comment.