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.

Sign up for GitHub

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

Addon-docs: Support for iframeWidth parameter on stories #9149

Closed
wants to merge 2 commits into from
Closed

Addon-docs: Support for iframeWidth parameter on stories #9149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d07fd28
Enables iFrameWidth parameter on Story components
alonsosfera Dec 13, 2019
b0330c3
Merge branch 'next' of https://github.com/storybookjs/storybook into …
alonsosfera Dec 13, 2019
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
4 changes: 3 additions & 1 deletion addons/docs/src/blocks/Anchor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@ export interface AnchorProps {
}

export const Anchor: FC<AnchorProps> = ({ storyId, children }) => (
<div id={anchorBlockIdFromId(storyId)}>{children}</div>
<div id={anchorBlockIdFromId(storyId)} style={{ textAlign: 'center' }}>
Copy link
Member

Choose a reason for hiding this comment

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

why text-align center?

Copy link
Author

Choose a reason for hiding this comment

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

I added this so no matter what size the iFrame has, the component will always be centered on the Docs Page...

Copy link
Author

Choose a reason for hiding this comment

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

Without it:
image

With it:
image

Copy link
Member

Choose a reason for hiding this comment

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

that visual outcome looks ok to me (I'm no designer), but this shouldn't be done with inline styles IMHO, I'm wondering if centering should be done with flexbox rather then text-align.

@shilman @domyen does the centering of the iframe seem like the right thing to do to you?

{children}
</div>
);
9 changes: 6 additions & 3 deletions addons/docs/src/blocks/Story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Object.keys(docsComponents).forEach(key => {

interface CommonProps {
height?: string;
width?: string;
inline?: boolean;
}

Expand Down Expand Up @@ -47,7 +48,7 @@ export const getStoryProps = (
const inputId = id === CURRENT_SELECTION ? currentId : id;
const previewId = inputId || mdxStoryNameToId[name];

const { height, inline } = props;
const { height, width, inline } = props;
const data = storyStore.fromId(previewId);
const { framework = null } = (data && data.parameters) || {};

Expand All @@ -61,6 +62,7 @@ export const getStoryProps = (
const {
inlineStories = inferInlineStories(framework),
iframeHeight = undefined,
iframeWidth = undefined,
prepareForInline = undefined,
} = docsParam;
const { storyFn = undefined, name: storyName = undefined } = data || {};
Expand All @@ -71,14 +73,15 @@ export const getStoryProps = (
`Story '${storyName}' is set to render inline, but no 'prepareForInline' function is implemented in your docs configuration!`
);
}

return {
const newVar = {
Copy link
Member

Choose a reason for hiding this comment

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

what is newVar?

Copy link
Author

Choose a reason for hiding this comment

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

After adding the width property to the StoryProps interface, TypeScript kept giving me the following error:
image

I kept getting the error whenever I would return the object like this:
image

After inserting the object into a constant and then returning it, the error cleared out.
image

inline: storyIsInline,
id: previewId,
storyFn: prepareForInline && storyFn ? () => prepareForInline(storyFn) : storyFn,
height: height || (storyIsInline ? undefined : iframeHeight),
width: width || (storyIsInline ? undefined : iframeWidth),
title: storyName,
};
return newVar;
};

const StoryContainer: FunctionComponent<StoryProps> = props => (
Expand Down
1 change: 1 addition & 0 deletions lib/components/src/blocks/Preview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const PreviewContainer = styled.div<PreviewProps>(
position: 'relative',
overflow: 'hidden',
margin: '25px 0 40px',
display: 'inline-block',
Copy link
Member

Choose a reason for hiding this comment

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

is this needed?

Copy link
Author

@alonsosfera alonsosfera Dec 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It is necessary in order for the preview container to adjust to the iFrame's width.

Without it, it would look like this:
image

Instead of this:
image

Copy link
Member

Choose a reason for hiding this comment

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

Using flexbox for this is a little more robust I think.

...getBlockBackgroundStyle(theme),
borderBottomLeftRadius: withSource && isExpanded && 0,
borderBottomRightRadius: withSource && isExpanded && 0,
Expand Down
20 changes: 13 additions & 7 deletions lib/components/src/blocks/Story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const MISSING_STORY = (id?: string) => (id ? `Story "${id}" doesn't exist.` : St
interface CommonProps {
title: string;
height?: string;
width?: string;
id: string;
}

Expand Down Expand Up @@ -53,8 +54,8 @@ const InlineZoomWrapper: FunctionComponent<{ scale: number }> = ({ scale, childr
);
};

const InlineStory: FunctionComponent<InlineStoryProps> = ({ storyFn, height, id }) => (
<div style={{ height }}>
const InlineStory: FunctionComponent<InlineStoryProps> = ({ storyFn, height, id, width }) => (
<div style={{ height, width }}>
<ZoomContext.Consumer>
{({ scale }) => (
<InlineZoomWrapper scale={scale}>
Expand All @@ -65,8 +66,13 @@ const InlineStory: FunctionComponent<InlineStoryProps> = ({ storyFn, height, id
</div>
);

const IFrameStory: FunctionComponent<IFrameStoryProps> = ({ id, title, height = '500px' }) => (
<div style={{ width: '100%', height }}>
const IFrameStory: FunctionComponent<IFrameStoryProps> = ({
id,
title,
height = '500px',
width = '950px',
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this completely optional?

Copy link
Author

Choose a reason for hiding this comment

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

That's actually just a default height and width. In case the IFrameStory component does not receive those parameters, it would just set it to 950x500 px...

Could you explain a little bit more about how this could be optional?

}) => (
<div style={{ width, height }}>
<ZoomContext.Consumer>
{({ scale }) => {
return (
Expand Down Expand Up @@ -96,15 +102,15 @@ const IFrameStory: FunctionComponent<IFrameStoryProps> = ({ id, title, height =
const Story: FunctionComponent<StoryProps> = props => {
const { error } = props as ErrorProps;
const { storyFn } = props as InlineStoryProps;
const { id, inline, title, height } = props;
const { id, inline, title, height, width } = props;

if (error) {
return <EmptyBlock>{error}</EmptyBlock>;
}
return inline ? (
<InlineStory id={id} storyFn={storyFn} title={title} height={height} />
<InlineStory id={id} storyFn={storyFn} title={title} height={height} width={width} />
) : (
<IFrameStory id={id} title={title} height={height} />
<IFrameStory id={id} title={title} height={height} width={width} />
);
};

Expand Down