Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
36 changes: 14 additions & 22 deletions code/core/src/components/components/Badge/Badge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,45 +31,37 @@ const BadgeWrapper = styled.div<BadgeProps>(
switch (status) {
case 'critical': {
return {
color: theme.color.critical,
background: theme.background.critical,
color: theme.fgColor.critical,
background: theme.bgColor.critical,
boxShadow: `inset 0 0 0 1px ${theme.borderColor.critical}`,
};
}
case 'negative': {
return {
color: theme.color.negativeText,
background: theme.background.negative,
boxShadow:
theme.base === 'light'
? `inset 0 0 0 1px ${transparentize(0.9, theme.color.negativeText)}`
: 'none',
color: theme.fgColor.negative,
background: theme.bgColor.negative,
boxShadow: `inset 0 0 0 1px ${theme.borderColor.negative}`,
};
}
case 'warning': {
return {
color: theme.color.warningText,
background: theme.background.warning,
boxShadow:
theme.base === 'light'
? `inset 0 0 0 1px ${transparentize(0.9, theme.color.warningText)}`
: 'none',
color: theme.fgColor.warning,
background: theme.bgColor.warning,
boxShadow: `inset 0 0 0 1px ${theme.borderColor.warning}`,
};
}
case 'neutral': {
return {
color: theme.textMutedColor,
background: theme.base === 'light' ? theme.background.app : theme.barBg,
color: theme.fgColor.muted,
background: theme.base === 'dark' ? theme.barBg : theme.background.app,
boxShadow: `inset 0 0 0 1px ${transparentize(0.8, theme.textMutedColor)}`,
};
}
case 'positive': {
return {
color: theme.color.positiveText,
background: theme.background.positive,
boxShadow:
theme.base === 'light'
? `inset 0 0 0 1px ${transparentize(0.9, theme.color.positiveText)}`
: 'none',
color: theme.fgColor.positive,
background: theme.bgColor.positive,
boxShadow: `inset 0 0 0 1px ${theme.borderColor.positive}`,
};
}
case 'active': {
Expand Down
9 changes: 5 additions & 4 deletions code/core/src/manager/components/sidebar/SearchResults.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { TrashIcon } from '@storybook/icons';
import type { ControllerStateAndHelpers } from 'downshift';
import { transparentize } from 'polished';
import { useStorybookApi } from 'storybook/manager-api';
import { styled } from 'storybook/theming';
import { styled, useTheme } from 'storybook/theming';

import { matchesKeyCode, matchesModifiers } from '../../keybinding';
import { statusMapping } from '../../utils/status';
import { getStatus } from '../../utils/status';
import { UseSymbol } from './IconSymbols';
import { NoResults } from './NoResults';
import { StatusLabel } from './StatusButton';
Expand Down Expand Up @@ -158,6 +158,7 @@ const Result: FC<
isHighlighted: boolean;
} & React.DetailedHTMLProps<React.LiHTMLAttributes<HTMLLIElement>, HTMLLIElement>
> = React.memo(function Result({ item, matches, onClick, ...props }) {
const theme = useTheme();
const click: MouseEventHandler<HTMLLIElement> = useCallback(
(event) => {
event.preventDefault();
Expand All @@ -171,12 +172,12 @@ const Result: FC<
if (api && props.isHighlighted && item.type === 'component') {
api.emit(PRELOAD_ENTRIES, { ids: [item.children[0]] }, { options: { target: item.refId } });
}
}, [props.isHighlighted, item]);
}, [api, props.isHighlighted, item]);

const nameMatch = matches.find((match: Match) => match.key === 'name');
const pathMatches = matches.filter((match: Match) => match.key === 'path');

const [icon] = item.status ? statusMapping[item.status] : [];
const [icon] = item.status ? getStatus(theme, item.status) : [];

return (
<ResultRow {...props} onClick={click}>
Expand Down
73 changes: 69 additions & 4 deletions code/core/src/manager/components/sidebar/Tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { styled, useTheme } from 'storybook/theming';

import type { Link } from '../../../components/components/tooltip/TooltipLinkList';
import { MEDIA_DESKTOP_BREAKPOINT } from '../../constants';
import { getGroupStatus, getMostCriticalStatusValue, statusMapping } from '../../utils/status';
import { getGroupStatus, getMostCriticalStatusValue, getStatus } from '../../utils/status';
import {
createId,
getAncestorIds,
Expand Down Expand Up @@ -214,6 +214,7 @@ const Node = React.memo<NodeProps>(function Node(props) {
onSelectStoryId,
api,
} = props;
const theme = useTheme();
const { isDesktop, isMobile, setMobileMenuOpen } = useLayout();
const { counts, statusesByValue } = useStatusSummary(item);

Expand Down Expand Up @@ -282,6 +283,71 @@ const Node = React.memo<NodeProps>(function Node(props) {
? useContextMenu(item, statusLinks, api)
: { node: null, onMouseEnter: () => {} };

if (
(item.type === 'story' &&
!('children' in item && item.children) &&
(!('subtype' in item) || item.subtype !== 'test')) ||
item.type === 'docs'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To sum it up, we want a status only on docs and stories entries that do not contain child tests. Is that so?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IIRC, status should be for:

  1. Groups (directory/component/story with tests)
  2. Stories and tests

The logic here is muddy to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Feel free to make changes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I do think this is inconsistent with line 418 on the same file, but the Vitest addon on the monorepo does not run on my machine so I can't investigate this right now. :(

) {
const LeafNode = item.type === 'docs' ? DocumentNode : StoryNode;

const statusValue = getMostCriticalStatusValue(
Object.values(statuses || {}).map((s) => s.value)
);
const [icon, textColor] = getStatus(theme, statusValue);

return (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a sanity check as I didn't expect you'd need to rework the tree structure. Is this new branch intentional or could it be caused by a merge autoresolution?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It could have been via merge. @ghengeveld helped me with the initial work and I don't have a solid grasp of this specific set of changes.

<LeafNodeStyleWrapper
key={id}
className="sidebar-item"
data-selected={isSelected}
data-ref-id={refId}
data-item-id={item.id}
data-parent-id={item.parent}
data-nodetype={item.type === 'docs' ? 'document' : 'story'}
data-highlightable={isDisplayed}
onMouseEnter={contextMenu.onMouseEnter}
>
<LeafNode
// @ts-expect-error (non strict)
style={isSelected ? {} : { color: textColor }}
Comment thread
MichaelArestad marked this conversation as resolved.
href={getLink(item, refId)}
id={id}
depth={isOrphan ? item.depth : item.depth - 1}
onClick={(event) => {
event.preventDefault();
onSelectStoryId(item.id);

if (isMobile) {
setMobileMenuOpen(false);
}
}}
{...(item.type === 'docs' && { docsMode })}
>
{(item.renderLabel as (i: typeof item, api: API) => React.ReactNode)?.(item, api) ||
item.name}
</LeafNode>
{isSelected && (
<SkipToContentLink asChild ariaLabel={false}>
<a href="#storybook-preview-wrapper">Skip to canvas</a>
</SkipToContentLink>
)}
{contextMenu.node}
{icon ? (
<StatusButton
ariaLabel={`Test status: ${statusValue.replace('status-value:', '')}`}
data-testid="tree-status-button"
type="button"
status={statusValue}
selectedItem={isSelected}
>
{icon}
</StatusButton>
) : null}
</LeafNodeStyleWrapper>
);
}

if (item.type === 'root') {
return (
<RootNode
Expand Down Expand Up @@ -327,7 +393,7 @@ const Node = React.memo<NodeProps>(function Node(props) {
}

const itemStatus = getMostCriticalStatusValue(Object.values(statuses || {}).map((s) => s.value));
const [itemIcon, itemColor] = statusMapping[itemStatus];
const [itemIcon, itemColor] = getStatus(theme, itemStatus);
const itemStatusButton = itemIcon ? (
<StatusButton
ariaLabel={`Test status: ${itemStatus.replace('status-value:', '')}`}
Expand All @@ -349,7 +415,7 @@ const Node = React.memo<NodeProps>(function Node(props) {
const { children = [] } = item;
const BranchNode = { component: ComponentNode, group: GroupNode, story: StoryNode }[item.type];
const status = getMostCriticalStatusValue([itemStatus, groupStatus?.[item.id]]);
const color = status ? statusMapping[status][1] : null;
const color = status ? getStatus(theme, status)[1] : null;
const showBranchStatus = status === 'status-value:error' || status === 'status-value:warning';

return (
Expand Down Expand Up @@ -454,7 +520,6 @@ const Node = React.memo<NodeProps>(function Node(props) {
setMobileMenuOpen(false);
}
}}
{...(item.type === 'docs' && { docsMode })}
>
{(item.renderLabel as (i: typeof item, api: API) => React.ReactNode)?.(item, api) ||
item.name}
Expand Down
1 change: 1 addition & 0 deletions code/core/src/manager/globals/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ export default {
'lighten',
'styled',
'themes',
'tokens',
'typography',
'useTheme',
'withTheme',
Expand Down
54 changes: 31 additions & 23 deletions code/core/src/manager/utils/status.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { type API_HashEntry, type StatusesByStoryIdAndTypeId } from 'storybook/i

import { CircleIcon } from '@storybook/icons';

import { styled } from 'storybook/theming';
import memoizerific from 'memoizerific';
import { type Theme, styled } from 'storybook/theming';

import { UseSymbol } from '../components/sidebar/IconSymbols';
import { getDescendantIds } from './tree';
Expand All @@ -31,28 +32,35 @@ export const statusPriority: StatusValue[] = [
'status-value:warning',
'status-value:error',
];
export const statusMapping: Record<StatusValue, [ReactElement | null, string | null]> = {
['status-value:unknown']: [null, null],
['status-value:pending']: [<LoadingIcons key="icon" />, 'currentColor'],
['status-value:success']: [
<svg key="icon" viewBox="0 0 14 14" width="14" height="14">
<UseSymbol type="success" />
</svg>,
'currentColor',
],
['status-value:warning']: [
<svg key="icon" viewBox="0 0 14 14" width="14" height="14">
<UseSymbol type="warning" />
</svg>,
'#A15C20',
],
['status-value:error']: [
<svg key="icon" viewBox="0 0 14 14" width="14" height="14">
<UseSymbol type="error" />
</svg>,
'#D43900',
],
};

// We might not want to make this a hook because it is used in the Tree after multiple returns.
// There could be scenarios where creating a story changes the type of an item (e.g. story now
// has children because it has a test child), so we could end up with rule of hooks violations.
export const getStatus = memoizerific(5)((theme: Theme, status: StatusValue) => {
const statusMapping: Record<StatusValue, [ReactElement | null, string | null]> = {
['status-value:unknown']: [null, null],
['status-value:pending']: [<LoadingIcons key="icon" />, 'currentColor'],
['status-value:success']: [
<svg key="icon" viewBox="0 0 14 14" width="14" height="14">
<UseSymbol type="success" />
</svg>,
'currentColor',
],
['status-value:warning']: [
<svg key="icon" viewBox="0 0 14 14" width="14" height="14">
<UseSymbol type="warning" />
</svg>,
theme.fgColor.warning,
],
['status-value:error']: [
<svg key="icon" viewBox="0 0 14 14" width="14" height="14">
<UseSymbol type="error" />
</svg>,
theme.fgColor.negative,
],
};
return statusMapping[status];
});

export const getMostCriticalStatusValue = (statusValues: StatusValue[]): StatusValue => {
return statusPriority.reduce(
Expand Down
67 changes: 66 additions & 1 deletion code/core/src/theming/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const color = {
darkest: '#2E3338',

// For borders
border: 'hsla(212, 50%, 30%, 0.15)',
border: 'hsl(212 50% 30% / 0.15)',

// Status
positive: '#66BF3C',
Expand Down Expand Up @@ -100,3 +100,68 @@ export const typography = {
code: 90,
},
};

export const tokens = {
light: {
fgColor: {
default: color.darkest,
muted: color.dark,
accent: color.secondary,
inverse: color.lightest,
// TODO: add 'disabled'
positive: '#427C27',
warning: '#7A4100',
negative: '#C23400',
critical: '#FFFFFF',
},
bgColor: {
default: color.lightest,
muted: background.app,
// TODO: add 'accent'? white or blue?
positive: '#F1FFEB',
warning: '#FFF7EB',
negative: '#FFF0EB',
critical: '#D13800',
},
borderColor: {
default: color.border,
muted: 'hsl(0 0% 0% / 0.1)',
inverse: 'hsl(0 0% 100% / 0.1)',
positive: '#BFE7AC',
warning: '#FFCE85',
negative: '#FFC3AD',
critical: 'hsl(16 100% 100% / 0)',
},
},
dark: {
fgColor: {
default: '#C9CCCF',
muted: '#95999D',
accent: '#479DFF',
inverse: '#1B1C1D',
// TODO: add 'disabled'
positive: '#86CE64',
warning: '#FFAD33',
negative: '#FF6933',
critical: '#FF6933',
},
bgColor: {
default: '#222325',
muted: '#1B1C1D',
// TODO: add 'accent'? white or blue?
positive: 'hsl(101 100% 100% / 0)',
warning: 'hsl(101 100% 100% / 0)',
negative: 'hsl(101 100% 100% / 0)',
critical: 'hsl(101 100% 100% / 0)',
Comment thread
MichaelArestad marked this conversation as resolved.
},
borderColor: {
default: 'hsl(0 0% 100% / 0.1)',
muted: 'hsl(0 0% 100% / 0.5)',
inverse: 'hsl(0 0% 0% / 0.1)',
positive: 'hsl(101 52% 64% / 0.15)',
warning: 'hsl(36 100% 64% / 0.15)',
negative: 'hsl(16 100% 64% / 0.15)',
critical: '#FF6933',
},
},
};
Loading