-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Experimental: Add API for addons to inject full-view 'pages' #23081
Conversation
children: ReactNode; | ||
} | ||
interface RoutePropsDefault { | ||
path: string | RegExp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It supports RegExp now!
@@ -10,25 +12,41 @@ export const VisuallyHidden = styled.div<VisuallyHiddenProps>(({ active }) => | |||
active ? { display: 'block' } : { display: 'none' } | |||
); | |||
|
|||
export const childrenToList = (children: any, selected: string) => | |||
export const childrenToList = (children: TabsProps['children']) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tabs component is due for a complete overhaul.
The API is has with the children being the interface was not a great idea of mine.
It was hard to get the title to show up and not lose state when the tab became active/inactive.
<S.Main {...mainProps} isFullscreen={!!mainProps.isFullscreen}> | ||
<S.Preview {...previewProps} hidden={viewMode === 'settings'}> | ||
<Preview id="main" /> | ||
</S.Preview> | ||
<Route path={/^\/story|docs\//} hideOnly> | ||
<S.Preview {...previewProps} hidden={false}> | ||
<Preview id="main" /> | ||
</S.Preview> | ||
|
||
<S.Panel {...panelProps} hidden={viewMode !== 'story'}> | ||
<Panel /> | ||
</S.Panel> | ||
<Route path="/story/" startsWith hideOnly> | ||
<S.Panel {...panelProps} hidden={false}> | ||
<Panel /> | ||
</S.Panel> | ||
</Route> | ||
</Route> | ||
|
||
{pages.map(({ key, route: Route, render: Content }) => ( | ||
<Route key={key}> | ||
{pages.map(({ key, route: Wrapper, render: Content }) => ( | ||
<Wrapper key={key}> | ||
<Content /> | ||
</Route> | ||
</Wrapper> | ||
))} | ||
</S.Main> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
settings
is now no longer some special string anymore.
<Content /> | ||
</Route> | ||
))} | ||
export const Mobile = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mobile view was such a mess, pages would show up, but then make it impossible to go back to canvas.
This needs more attentions and proper testing.
I suspect there's a range of pre-existing bugs still here
@@ -25,6 +26,115 @@ export const Default = () => { | |||
); | |||
}; | |||
|
|||
export const JSXTitles = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Demonstrating addon titles being a React.FC
Extracted into steps PRs:
Router change:
#23292
Tabs accepting title as
FC
:#23288
Addon interactions overhaul using title as
FC
anduseAddonState
:#23291
Other addons:
#23298
The big bang:
#23307