Skip to content

Angular: Correctly set the 'required' argType #28718

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

Open
wants to merge 18 commits into
base: next
Choose a base branch
from
Open
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
27 changes: 27 additions & 0 deletions code/frameworks/angular/src/client/docs/compodoc.test.ts
Original file line number Diff line number Diff line change
@@ -12,6 +12,33 @@ const makeProperty = (compodocType?: string) => ({

const getDummyCompodocJson = () => {
return {
components: [
{
name: 'ButtonComponent',
type: 'component',
propertiesClass: [],
inputsClass: [
{
required: true,
name: 'label',
defaultValue: "'Button'",
type: 'string',
decorators: [],
},
{
name: 'primary',
defaultValue: 'false',
deprecated: false,
deprecationMessage: '',
line: 23,
type: 'boolean',
decorators: [],
},
],
outputsClass: [],
methodsClass: [],
},
],
miscellaneous: {
typealiases: [
{
18 changes: 12 additions & 6 deletions code/frameworks/angular/src/client/docs/compodoc.ts
Original file line number Diff line number Diff line change
@@ -21,6 +21,14 @@ export const isMethod = (methodOrProp: Method | Property): methodOrProp is Metho
return (methodOrProp as Method).args !== undefined;
};

export const isRequired = (item: Property): boolean => {
return item.hasOwnProperty('required') && item.required;
};

export const setRequiredProperty = (property: Property) => {
return isRequired(property) ? { required: true } : {};
};

export const setCompodocJson = (compodocJson: CompodocJson) => {
global.__STORYBOOK_COMPODOC_JSON__ = compodocJson;
};
@@ -139,13 +147,13 @@ const extractEnumValues = (compodocType: any) => {
export const extractType = (property: Property, defaultValue: any): SBType => {
const compodocType = property.type || extractTypeFromValue(defaultValue);
switch (compodocType) {
case 'string':
case 'boolean':
case 'string':
case 'number':
return { name: compodocType };
return { name: compodocType, ...setRequiredProperty(property) };
case undefined:
case null:
return { name: 'other', value: 'void' };
return { name: 'other', value: 'void', ...setRequiredProperty(property) };
default: {
const resolvedType = resolveTypealias(compodocType);
const enumValues = extractEnumValues(resolvedType);
@@ -246,8 +254,7 @@ export const extractArgTypesFromData = (componentData: Class | Directive | Injec
? { name: 'other', value: 'void' }
: extractType(item as Property, defaultValue);
const action = section === 'outputs' ? { action: item.name } : {};

const argType = {
const argType: InputType = {
name: item.name,
description: item.rawdescription || item.description,
type,
@@ -256,7 +263,6 @@ export const extractArgTypesFromData = (componentData: Class | Directive | Injec
category: section,
type: {
summary: isMethod(item) ? displaySignature(item) : item.type,
required: isMethod(item) ? false : !item.optional,
},
defaultValue: { summary: defaultValue },
},
1 change: 1 addition & 0 deletions code/frameworks/angular/src/client/docs/types.ts
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@ export interface Property {
type: string;
optional: boolean;
defaultValue?: string;
required?: boolean;
description?: string;
rawdescription?: string;
jsdoctags?: JsDocTag[];
4 changes: 2 additions & 2 deletions code/frameworks/angular/template/cli/button.component.ts
Original file line number Diff line number Diff line change
@@ -33,8 +33,8 @@ export class ButtonComponent {
*
* @required
*/
@Input()
label = 'Button';
@Input({ required: true })
label: string;

/** Optional click handler */
@Output()
Original file line number Diff line number Diff line change
@@ -31,8 +31,8 @@ export default class FrameworkButtonComponent {
*
* @required
*/
@Input()
label = 'Button';
@Input({ required: true })
label: string;

/** Optional click handler */
@Output()
24 changes: 17 additions & 7 deletions code/lib/blocks/src/components/ArgsTable/ArgControl.tsx
Original file line number Diff line number Diff line change
@@ -3,6 +3,8 @@ import React, { useCallback, useEffect, useState } from 'react';

import { Link } from 'storybook/internal/components';

import type { StrictInputType } from '@storybook/csf';

import {
BooleanControl,
ColorControl,
@@ -13,17 +15,18 @@ import {
OptionsControl,
RangeControl,
TextControl,
isControlObject,
} from '../../controls';
import type { ArgType, Args } from './types';
import type { Args } from './types';

export interface ArgControlProps {
row: ArgType;
row: StrictInputType;
arg: any;
updateArgs: (args: Args) => void;
isHovered: boolean;
}

const Controls: Record<string, FC> = {
const Controls: Record<string, FC<Record<string, any>>> = {
array: ObjectControl,
object: ObjectControl,
boolean: BooleanControl,
@@ -68,8 +71,10 @@ export const ArgControl: FC<ArgControlProps> = ({ row, arg, updateArgs, isHovere
const onBlur = useCallback(() => setFocused(false), []);
const onFocus = useCallback(() => setFocused(true), []);

if (!control || control.disable) {
const canBeSetup = control?.disable !== true && row?.type?.name !== 'function';
if (!control || (isControlObject(control) && control.disable)) {
const canBeSetup = isControlObject(control)
? control.disable !== true && row?.type?.name !== 'function'
: false;
return isHovered && canBeSetup ? (
<Link href="https://storybook.js.org/docs/essentials/controls" target="_blank" withArrow>
Setup controls
@@ -81,6 +86,11 @@ export const ArgControl: FC<ArgControlProps> = ({ row, arg, updateArgs, isHovere
// row.name is a display name and not a suitable DOM input id or name - i might contain whitespace etc.
// row.key is a hash key and therefore a much safer choice
const props = { name: key, argType: row, value: boxedValue.value, onChange, onBlur, onFocus };
const Control = Controls[control.type] || NoControl;
return <Control {...props} {...control} controlType={control.type} />;
const Control = typeof control !== 'string' ? (Controls[control.type] ?? NoControl) : NoControl;

if (typeof control === 'string') {
return <Control {...props} />;
} else {
return <Control {...props} {...control} controlType={control.type} />;
}
};
8 changes: 6 additions & 2 deletions code/lib/blocks/src/components/ArgsTable/ArgRow.stories.tsx
Original file line number Diff line number Diff line change
@@ -2,6 +2,8 @@ import React from 'react';

import { ResetWrapper } from 'storybook/internal/components';

import type { InputType } from '@storybook/csf';

import { ArgRow } from './ArgRow';
import { TableWrapper } from './ArgsTable';

@@ -31,6 +33,7 @@ export const String = {
description: 'someString description',
type: {
required: true,
name: 'string',
},
control: {
type: 'text',
@@ -43,7 +46,7 @@ export const String = {
summary: 'reallylongstringnospaces',
},
},
},
} satisfies InputType,
},
};

@@ -127,6 +130,7 @@ export const Number = {
description: 'someNumber description',
type: {
required: false,
name: 'number',
},
table: {
type: {
@@ -139,7 +143,7 @@ export const Number = {
control: {
type: 'number',
},
},
} satisfies InputType,
},
};

20 changes: 11 additions & 9 deletions code/lib/blocks/src/components/ArgsTable/ArgRow.tsx
Original file line number Diff line number Diff line change
@@ -5,17 +5,19 @@ import { codeCommon } from 'storybook/internal/components';
import type { CSSObject } from 'storybook/internal/theming';
import { styled } from 'storybook/internal/theming';

import type { InputType, SBType } from '@storybook/csf';

import Markdown from 'markdown-to-jsx';
import { transparentize } from 'polished';

import type { ArgControlProps } from './ArgControl';
import { ArgControl } from './ArgControl';
import { ArgJsDoc } from './ArgJsDoc';
import { ArgValue } from './ArgValue';
import type { ArgType, Args, TableAnnotation } from './types';
import type { Args } from './types';

interface ArgRowProps {
row: ArgType;
row: InputType;
arg: any;
updateArgs?: (args: Args) => void;
compact?: boolean;
@@ -78,9 +80,9 @@ const StyledTd = styled.td<{ expandable: boolean }>(({ theme, expandable }) => (
paddingLeft: expandable ? '40px !important' : '20px !important',
}));

const toSummary = (value: any) => {
const toSummary = (value: InputType['type']): { summary: string } => {
if (!value) {
return value;
return value as any;
}
const val = typeof value === 'string' ? value : value.name;
return { summary: val };
@@ -90,10 +92,10 @@ export const ArgRow: FC<ArgRowProps> = (props) => {
const [isHovered, setIsHovered] = useState(false);
const { row, updateArgs, compact, expandable, initialExpandedArgs } = props;
const { name, description } = row;
const table = (row.table || {}) as TableAnnotation;
const type = table.type || toSummary(row.type);
const table = row.table || {};
const tableType = table.type || toSummary(row.type);
const defaultValue = table.defaultValue || row.defaultValue;
const required = row.type?.required;
const required = typeof row.type === 'string' ? false : row.type?.required;
const hasDescription = description != null && description !== '';

return (
@@ -112,13 +114,13 @@ export const ArgRow: FC<ArgRowProps> = (props) => {
{table.jsDocTags != null ? (
<>
<TypeWithJsDoc hasDescription={hasDescription}>
<ArgValue value={type} initialExpandedArgs={initialExpandedArgs} />
<ArgValue value={tableType} initialExpandedArgs={initialExpandedArgs} />
</TypeWithJsDoc>
<ArgJsDoc tags={table.jsDocTags} />
</>
) : (
<Type hasDescription={hasDescription}>
<ArgValue value={type} initialExpandedArgs={initialExpandedArgs} />
<ArgValue value={tableType} initialExpandedArgs={initialExpandedArgs} />
</Type>
)}
</td>
7 changes: 3 additions & 4 deletions code/lib/blocks/src/components/ArgsTable/ArgValue.tsx
Original file line number Diff line number Diff line change
@@ -4,15 +4,14 @@ import React, { useState } from 'react';
import { SyntaxHighlighter, WithTooltipPure, codeCommon } from 'storybook/internal/components';
import { styled } from 'storybook/internal/theming';

import type { InputType, SBType } from '@storybook/csf';
import { ChevronSmallDownIcon, ChevronSmallUpIcon } from '@storybook/icons';

import { uniq } from 'es-toolkit/compat';
import memoize from 'memoizerific';

import type { PropSummaryValue } from './types';

interface ArgValueProps {
value?: PropSummaryValue;
value?: InputType['table']['type'];
initialExpandedArgs?: boolean;
}

@@ -22,7 +21,7 @@ interface ArgTextProps {
}

interface ArgSummaryProps {
value: PropSummaryValue;
value: InputType['table']['type'];
initialExpandedArgs?: boolean;
}

30 changes: 19 additions & 11 deletions code/lib/blocks/src/components/ArgsTable/ArgsTable.tsx
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ import { once } from 'storybook/internal/client-logger';
import { IconButton, Link, ResetWrapper } from 'storybook/internal/components';
import { styled } from 'storybook/internal/theming';

import { includeConditionalArg } from '@storybook/csf';
import { type InputType, includeConditionalArg } from '@storybook/csf';
import { DocumentIcon, UndoIcon } from '@storybook/icons';

import { pickBy } from 'es-toolkit/compat';
@@ -16,7 +16,7 @@ import { ArgRow } from './ArgRow';
import { Empty } from './Empty';
import { SectionRow } from './SectionRow';
import { Skeleton } from './Skeleton';
import type { ArgType, ArgTypes, Args, Globals } from './types';
import type { Args, Globals } from './types';

export const TableWrapper = styled.table<{
compact?: boolean;
@@ -167,7 +167,7 @@ export const TableWrapper = styled.table<{
},
}));

const StyledIconButton = styled(IconButton as any)(({ theme }) => ({
const StyledIconButton = styled(IconButton as any)(() => ({
margin: '-4px -12px -4px 0',
}));

@@ -182,12 +182,18 @@ export enum ArgsTableError {
}

export type SortType = 'alpha' | 'requiredFirst' | 'none';
type SortFn = (a: ArgType, b: ArgType) => number;
type SortFn = (a: InputType, b: InputType) => number;

const sortFns: Record<SortType, SortFn | null> = {
alpha: (a: ArgType, b: ArgType) => a.name.localeCompare(b.name),
requiredFirst: (a: ArgType, b: ArgType) =>
Number(!!b.type?.required) - Number(!!a.type?.required) || a.name.localeCompare(b.name),
alpha: (a: InputType, b: InputType) => a.name.localeCompare(b.name),
requiredFirst: (a: InputType, b: InputType) => {
const bType = b.type;
const aType = a.type;
const isBTypeRequired = !!(typeof bType !== 'string' && bType.required);
const isATypeRequired = !!(typeof aType !== 'string' && aType.required);

return Number(isBTypeRequired) - Number(isATypeRequired) || a.name.localeCompare(b.name);
},
none: undefined,
};

@@ -202,7 +208,9 @@ export interface ArgsTableOptionProps {
sort?: SortType;
}
interface ArgsTableDataProps {
rows: ArgTypes;
rows: {
[key: string]: InputType;
};
args?: Args;
globals?: Globals;
}
@@ -218,7 +226,7 @@ export interface ArgsTableLoadingProps {
export type ArgsTableProps = ArgsTableOptionProps &
(ArgsTableDataProps | ArgsTableErrorProps | ArgsTableLoadingProps);

type Rows = ArgType[];
type Rows = InputType[];
type Subsection = Rows;
type Section = {
ungrouped: Rows;
@@ -230,7 +238,7 @@ type Sections = {
sections: Record<string, Section>;
};

const groupRows = (rows: ArgType, sort: SortType) => {
const groupRows = (rows: { [key: string]: InputType }, sort: SortType) => {
const sections: Sections = { ungrouped: [], ungroupedSubsections: {}, sections: {} };

if (!rows) {
@@ -298,7 +306,7 @@ const groupRows = (rows: ArgType, sort: SortType) => {
* preview in `prepareStory`, and that exception will be bubbled up into the UI in a red screen.
* Nevertheless, we log the error here just in case.
*/
const safeIncludeConditionalArg = (row: ArgType, args: Args, globals: Globals) => {
const safeIncludeConditionalArg = (row: InputType, args: Args, globals: Globals) => {
try {
return includeConditionalArg(row, args, globals);
} catch (err) {
Loading