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

Core: Add STORY_SPECIFIED event for initial sidebar selection/URL #11766

Merged
merged 4 commits into from
Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions lib/api/src/modules/stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
STORY_CHANGED,
SELECT_STORY,
SET_STORIES,
CURRENT_STORY_WAS_SET,
STORY_SPECIFIED,
} from '@storybook/core-events';
import deprecate from 'util-deprecate';

Expand Down Expand Up @@ -328,11 +328,8 @@ export const init: ModuleFn = ({

const initModule = () => {
// On initial load, the local iframe will select the first story (or other "selection specifier")
// and emit CURRENT_STORY_WAS_SET with the id. We need to ensure we respond to this change.
// Later when we change story via the manager (or SELECT_STORY below), we'll already be at the
// correct path before CURRENT_STORY_WAS_SET is emitted, so this is less important (the navigate is a no-op)
// Note this is the case for refs also.
fullAPI.on(CURRENT_STORY_WAS_SET, function handler({
// and emit STORY_SPECIFIED with the id. We need to ensure we respond to this change.
fullAPI.on(STORY_SPECIFIED, function handler({
storyId,
viewMode,
}: {
Expand All @@ -344,8 +341,13 @@ export const init: ModuleFn = ({

if (fullAPI.isSettingsScreenActive()) return;

if (sourceType === 'local' && storyId && viewMode) {
navigate(`/${viewMode}/${storyId}`);
if (sourceType === 'local') {
// Special case -- if we are already at the story being specified (i.e. the user started at a given story),
// we don't need to change URL. See https://github.com/storybookjs/storybook/issues/11677
const state = store.getState();
if (state.storyId !== storyId || state.viewMode !== viewMode) {
navigate(`/${viewMode}/${storyId}`);
}
}
});

Expand Down
23 changes: 19 additions & 4 deletions lib/api/src/tests/stories.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
UPDATE_STORY_ARGS,
RESET_STORY_ARGS,
SET_STORIES,
CURRENT_STORY_WAS_SET,
STORY_SPECIFIED,
} from '@storybook/core-events';
import { EventEmitter } from 'events';

Expand Down Expand Up @@ -329,7 +329,7 @@ describe('stories API', () => {
});

// Can't currently run these tests as cannot set this on the events
describe('CURRENT_STORY_WAS_SET event', () => {
describe('STORY_SPECIFIED event', () => {
it('navigates to the story', async () => {
const navigate = jest.fn();
const api = Object.assign(new EventEmitter(), {
Expand All @@ -341,11 +341,26 @@ describe('stories API', () => {
const { init } = initStories({ store, navigate, provider, fullAPI: api });

init();
api.emit(CURRENT_STORY_WAS_SET, { storyId: 'a--1', viewMode: 'story' });
api.emit(STORY_SPECIFIED, { storyId: 'a--1', viewMode: 'story' });

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

it('DOES not navigate if the story was already selected', async () => {
const navigate = jest.fn();
const api = Object.assign(new EventEmitter(), {
isSettingsScreenActive() {
return true;
},
});
const store = createMockStore({ viewMode: 'story', storyId: 'a--1' });
initStories({ store, navigate, provider, fullAPI: api });

api.emit(STORY_SPECIFIED, { storyId: 'a--1', viewMode: 'story' });

expect(navigate).not.toHaveBeenCalled();
});

it('DOES not navigate if a settings page was selected', async () => {
const navigate = jest.fn();
const api = Object.assign(new EventEmitter(), {
Expand All @@ -356,7 +371,7 @@ describe('stories API', () => {
const store = createMockStore({ viewMode: 'settings', storyId: 'about' });
initStories({ store, navigate, provider, fullAPI: api });

api.emit(CURRENT_STORY_WAS_SET, { storyId: 'a--1', viewMode: 'story' });
api.emit(STORY_SPECIFIED, { storyId: 'a--1', viewMode: 'story' });

expect(navigate).not.toHaveBeenCalled();
});
Expand Down
26 changes: 26 additions & 0 deletions lib/client-api/src/story_store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1288,4 +1288,30 @@ describe('preview.story_store', () => {
expect(onCurrentStoryWasSet).toHaveBeenCalled();
});
});

describe('STORY_SPECIFIED', () => {
it('is emitted when configuration ends if a specifier was set', () => {
const onStorySpecified = jest.fn();
channel.on(Events.STORY_SPECIFIED, onStorySpecified);
const store = new StoryStore({ channel });
addStoryToStore(store, 'kind-1', 'story-1.1', () => 0);
store.setSelectionSpecifier({ storySpecifier: '*', viewMode: 'story' });

store.finishConfiguring();
expect(onStorySpecified).toHaveBeenCalled();
});

it('is NOT emitted when setSelection is called', () => {
const onStorySpecified = jest.fn();
channel.on(Events.STORY_SPECIFIED, onStorySpecified);
const store = new StoryStore({ channel });
addStoryToStore(store, 'kind-1', 'story-1.1', () => 0);
store.setSelectionSpecifier({ storySpecifier: '*', viewMode: 'story' });
store.finishConfiguring();

onStorySpecified.mockClear();
store.setSelection({ storyId: 'a--1', viewMode: 'story' });
expect(onStorySpecified).not.toHaveBeenCalled();
});
});
});
2 changes: 2 additions & 0 deletions lib/client-api/src/story_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,12 @@ export default class StoryStore {

if (foundStory) {
this.setSelection({ storyId: foundStory.id, viewMode });
this._channel.emit(Events.STORY_SPECIFIED, { storyId: foundStory.id, viewMode });
}
}

// If we didn't find a story matching the specifier, we always want to emit CURRENT_STORY_WAS_SET anyway
// in order to tell the StoryRenderer to render something (a "missing story" view)
if (!foundStory && this._channel) {
this._channel.emit(Events.CURRENT_STORY_WAS_SET, this._selection);
}
Expand Down
11 changes: 7 additions & 4 deletions lib/core-events/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
enum events {
CHANNEL_CREATED = 'channelCreated',
// When the preview boots, the first story is chosen via a selection specifier
STORY_SPECIFIED = 'storySpecified',
// Emitted by the preview whenever the list of stories changes (in batches)
SET_STORIES = 'setStories',
// Set the current story selection in the preview
SET_CURRENT_STORY = 'setCurrentStory',
// The current story changed due to the above
CURRENT_STORY_WAS_SET = 'currentStoryWasSet',
// Emitted by the preview whenever the list of stories changes (in batches)
SET_STORIES = 'setStories',
// Force the current story to re-render
FORCE_RE_RENDER = 'forceReRender',
// The next 6 events are emitted by the StoryRenderer when rendering the current story
Expand Down Expand Up @@ -44,8 +46,10 @@ export default events;
// This is the preferred method
export const {
CHANNEL_CREATED,
SET_CURRENT_STORY,
STORY_SPECIFIED,
SET_STORIES,
SET_CURRENT_STORY,
CURRENT_STORY_WAS_SET,
FORCE_RE_RENDER,
STORY_CHANGED,
STORY_UNCHANGED,
Expand All @@ -67,5 +71,4 @@ export const {
SHARED_STATE_CHANGED,
SHARED_STATE_SET,
NAVIGATE_URL,
CURRENT_STORY_WAS_SET,
} = events;