Draft: Add story system
How about adding a story system to the library?
Currently we are defining the usage in static docs page but with stories we could display all the button sates in an interactive manner and let users play with the components.
I was working on this branch a while back and since I'm cleaning some local stuff up, I decided to publish it. I've played here a bit with histoire.dev to check it out and see how well we could integrate it. It's still in a very early stage but it looks promising. I've been having some major difficulties with integrating Vue/Vite projects with storybook in the past, hence the decision to play with Vite-native tool.
To run histoire:
yarn story:devMerge request reports
Activity
added Type::Enhancement label
requested review from @Sporiff and @georgkrause
assigned to @wvffle
@wvffle I am not sure if it makes sense to do a review here, its more about the question weather or not we want a story system, isn't it? Since this is about how we document or showcase the thing, its probably something @Sporiff has more useful to say.
I once had to maintain a story system at work, which was pain, but it was also quite useful because we were able to build frontend parts to let the customer review it and only if it got accepted we build the backend for it. I am wondering if this is a useful case for us, since we don't have customers in that sense.
So for me its all about evaluating if this is something useful we can maintain without a lot of effort. Considering histoire is still in development I'd expect us to change quite a few thing in the near future, is this worth it?
@georgkrause @wvffle My opinion on these things is always: more documentation is worth an administrative tradeoff. Story systems improve onboarding for developers and visibility for users, so I'm all in favor.
Looking at what this produces, we'd need to update the documentation to fit the supported syntax. I don't think this is too much of a lift, though.
I am not sure if it makes sense to do a review here, its more about the question weather or not we want a story system, isn't it?
@georgkrause of course it is. Since I wasn't sure if we even wanted it and I didn't want to waste time on it, I pushed what I already had and added you both as the reviewers to engage discussion.
- src/components/button/Button.story.vue 0 → 100644
79 </template> 80 81 <template #controls="{ state }"> 82 <div class="hst-label">Style:</div> 83 <HstSelect v-model="state.color" title="color" :options="$fwColors" /> 84 <HstCheckbox v-model="state.outline" title="outline" /> 85 <HstCheckbox v-model="state.shadow" title="shadow" /> 86 <HstCheckbox v-model="state.round" title="round" /> 87 88 <div class="hst-label">State:</div> 89 <HstCheckbox v-model="state.isActive" title="isActive" /> 90 <HstCheckbox v-model="state.isLoading" title="isLoading" /> 91 92 <div class="hst-label">Icon:</div> 93 <HstText v-model="state.icon" title="icon" /> 94 </template> - Comment on lines +81 to +94
This is one of disadvantages of Histoire right now. Even though there is a system to automatically detect component props and to create respective controls built in to the product, we need to define our custom ones. Histoire links the state of the component to the auto generated controls only one way:
controls->component. This means that the controls are uninitialized despite the props provided by the specific variant. To overcome this, I recreated the controls, bound the state to the<FwButton>component and passedauto-props-disabledto the<Variant>. Now it works like a charm. -
@wvffle But this creates a lot of duplications in the story, we could just have one button and let people play around with the props instead of listing different kinds of buttons. I think this is even more easy to understand.
-
There are both advantages and disadvantages of either approach. Maybe instead of creating variants for each and every possible state, we could keep things very simple and create one story per one variant? By variants here, I mean like each card variant (radio, album, artist, basic...)
-
@georgkrause what do you say?
-
@wvffle Sorry for the late reply, I somehow missed the update here. I am always in favor of simple solutions, we still have quite some things to do here to deploy it somewhere, so lets keep it simple for now. We can test it and extend later if needed.
- src/components/button/Button.story.vue 0 → 100644
63 { 64 title: 'Icon without content', 65 text: undefined, 66 state: () => ({ 67 icon: 'bi-search' 68 }) 69 }, 70 ]) 71 </script> 72 73 <template> 74 <Story title="<FwButton>"> 75 <Variant v-for="v in variants" :key="v.title" :init-state="v.state" :title="v.title" auto-props-disabled> 76 <template #="{ state }"> 77 <FwButton v-if="v.text" @click="logEvent('click', $event)" v-bind="$fwOmitHstProps(state)">{{ v.text }}</FwButton> 78 <FwButton v-else @click="logEvent('click', $event)" v-bind="$fwOmitHstProps(state)" /> - Comment on lines +77 to +78
The
v-ifhere is required here due to the implementation of<FwButton>:<FwButton icon="...">text</FwButton> <!-- icon button with text --> <FwButton icon="..."></FwButton> <!-- icon button with text --> <FwButton icon="..." /> <!-- square icon button -->Note that empty text (2nd case) is still a valid slot (in Vue terms) even though it is empty.
<FwButton>checks internally if the slot is present or not and chooses which width to render.
- src/components/button/Button.story.vue 0 → 100644
62 }, 63 { 64 title: 'Icon without content', 65 text: undefined, 66 state: () => ({ 67 icon: 'bi-search' 68 }) 69 }, 70 ]) 71 </script> 72 73 <template> 74 <Story title="<FwButton>"> 75 <Variant v-for="v in variants" :key="v.title" :init-state="v.state" :title="v.title" auto-props-disabled> 76 <template #="{ state }"> 77 <FwButton v-if="v.text" @click="logEvent('click', $event)" v-bind="$fwOmitHstProps(state)">{{ v.text }}</FwButton> -
Histoire checks for bound properties of the component, so I've used
v-bindhere to easily pass all the properties of the state. Note that I need to strip the state from some Histoire internal stuff that is added. Normally that wouldn't be an issue as Histoire expects us to bind the state props one by one (<FwButton :icon="state.icon" :color="state.color" ...>) but I see it as an overhead and sincev-bindis available to do this automatically, why not?This is also subject that I hope to change with future Histoire releases.
- src/components/button/Button.story.vue 0 → 100644
98 99 <docs lang="md"> 100 # Button 101 102 Buttons are UI elements that users can interact with to perform actions. Funkwhale uses buttons in many contexts. 103 104 ## Button styles 105 Buttons come in different styles that you can use depending on the location or action of the button. 106 107 ### Primary 108 109 Primary buttons represent **positive** actions such as uploading, confirming, and accepting changes. 110 111 ::: info 112 This is the default type. If you don't specify a type, a primary button is rendered. 113 ::: -
@Sporiff ping^^
@wvffle @georgkrause This syntax is specific to Vitepress, but it's purely visual. Anything like this can be restructured in the documentation to convey the same information without any visual flair.
-
@georgkrause, @Sporiff There are some issues with the state of the variants, so we need to work around them in some unpleasant ways. I noted in review comments this behavior and explained some workarounds
Edited by Kasper Seweryn
-
@wvffle Suggestion for the workflow here: You finish the work at the story system and afterwards we can create a follow-up issue for the deployment which I can take over.
removed review request for @georgkrause
@wvffle What's the state of this one? Would you like me to work through the markdown content or is there something holding this back?
-
There's nothing holding it back. I will tackle this MR shortly and simplify the stories as suggested here !65 (comment 56314)
Hi,
We're posting this message because this merge request meets the following criteria:
- No activity in the past 60 days (since 2023-11-23T10:53:57.659Z)
We'd like to ask you to help us out and determine how we should act on this merge request:
- is it still a WIP?
- is it still relevant?
- is it missing some reviews?
- do you need help or guidance to proceed?
Some community members have been pinged and will have a look at it too.
Thanks for your help!
❤ /cc @funkwhale/steering
added Status::Input wanted label
removed Status::Input wanted label
Hi,
We're posting this message because this merge request meets the following criteria:
- No activity in the past 60 days (since 2024-01-24T12:05:37.866Z)
We'd like to ask you to help us out and determine how we should act on this merge request:
- is it still a WIP?
- is it still relevant?
- is it missing some reviews?
- do you need help or guidance to proceed?
Some community members have been pinged and will have a look at it too.
Thanks for your help!
❤ /cc @funkwhale/steering
added Status::Input wanted label
removed Status::Input wanted label
