refactor(storybook): replace fake knobs with controls for simple stories #9385
Conversation
…eciado88/9084-set-up-controls-for-simple-storybook-stories
There was a problem hiding this comment.
This looks great, @aPreciado88! 🤘😎🤘
The main thing that needs to be addressed is the update to packages/calcite-components/src/components/interfaces.ts affecting component APIs. The rest can be tackled in follow-up PRs.
| heading="Heading" | ||
| description="Description for item" | ||
| icon-start="banana" | ||
| icon-end="" |
There was a problem hiding this comment.
Let's omit empty string attributes. Applies to similar attributes in other stories.
| interface ActionBarArgs { | ||
| expandDisabled: boolean; | ||
| expanded: boolean; | ||
| position: string; |
There was a problem hiding this comment.
You might be able to build the args interface from a component's class:
import { ActionBar } from "./action-bar";
// ...
type ActionBarArgs = Pick<ActionBar, "expanded" | "expandDisabled" | "position">;Applies to all component args.
| export const simple = (args: ActionBarArgs): string => html` | ||
| <calcite-action-bar | ||
| ${args.expandDisabled ? "expand-disabled" : ""} | ||
| ${args.expanded ? "expanded" : ""} |
There was a problem hiding this comment.
Can you add a util to handle booleans? We had something similar before I swapped it out with the fake knobs. 😅
|
|
||
| darkModeRTL_TestOnly.parameters = { themes: modesDarkDefault }; | ||
|
|
||
| export const adjacentTooltipsOpenQuickly = (): string => | ||
| html`<div style="display:flex; height:500px; width: 200px;"> | ||
| export const adjacentTooltipsOpenQuickly = (): string => html` |
| export type LogicalFlowPosition = "inline-start" | "inline-end" | "block-start" | "block-end"; | ||
| export type ModeClass = "calcite-mode-light" | "calcite-mode-dark" | "calcite-mode-auto"; | ||
| export type ModeName = "light" | "dark" | "auto"; | ||
| export type Position = "start" | "end"; | ||
| export type SelectionAppearance = "border" | "icon"; | ||
| export type Position = "start" | "end" | "top" | "bottom"; |
There was a problem hiding this comment.
This will affect types of existing component properties. For example, components using Position will now show additional options allowed that might not be supported/applicable.
Can you introduce an interfaces.ts file in .storybook that exports and augments these types? We can update the types and component props afterwards.
| --calcite-color-border-3: ${color("--calcite-color-border-3", "#dfdfdf")}; | ||
| --calcite-color-border-input: ${color("--calcite-color-border-input", "#949494")}; | ||
| --calcite-ui-icon-color: ${color("--calcite-ui-icon-color", "currentColor")}; | ||
| --calcite-color-brand: ${"#007ac2"}; |
There was a problem hiding this comment.
Can this have controls as well? This one might be useful to keep interactive for testing. cc @SkyeSeitz @ashetland
| <calcite-action | ||
| slot="actions-end" | ||
| label="click-me" | ||
| onclick="console.log('clicked');" |
There was a problem hiding this comment.
Sidebar: we should remove this and similar on<event> attributes from stories.
There was a problem hiding this comment.
Awesome! Thanks for moving the interface changes to the Storybook folder.
The rest of my previous comments can be tackled in follow-up PRs, so I'll approve and leave it up to you.
Before merging, can you:
- resolve merge conflicts
- add the
ready for visual snapshotslabel to run screenshot tests - create a follow-up issue to consolidate the storybook & component types
?
|
@jcfranco Thank you for approving it! I'm going through your other comments and trying to get as much as I can in, so it can go out next week. I'll create follow-up issues for the remaining work, as you suggested. |
…eciado88/9084-set-up-controls-for-simple-storybook-stories
…eciado88/9084-set-up-controls-for-simple-storybook-stories
|
@jcfranco I addressed all of the comments, only the interfaces comment is pending, I'll create a separate issue to work on this. I also created this issue to work on consolidating the augmented types. |
|
@ashetland @SkyeSeitz Could you please give this a review? |
|
Controls are looking good to me! |
…eciado88/9084-set-up-controls-for-simple-storybook-stories
* main: docs: update component READMEs (#9446) build: update browserslist db (#9445) ci(renovate): remove noisy sections from the PR body (#9441) refactor(storybook): replace fake knobs with controls for simple stories (#9385) build: remove unnecessary dist files before publishing (#9439) build(deps): update dependency axe-core to v4.9.1 (#9436) ci: combine related actions into single workflow file (#9434) build(deps): update dependency @floating-ui/dom to v1.6.5 (#9435) build(deps): update angular monorepo to v17.3.10 (#9431) ci: upload dist to github releases (#9428) ci: skip chromatic for release and doc update PRs (#9430) ci: enable npm provenance statements (#9429)
Related Issue: 9084
Summary
-Replaces knobs with controls for
simplestories.-Removes knobs used in all other stories.
-Replaces usage of
createComponentHTMLand related utils with HTML.