Skip to content
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

Setting type "radios" does not work in Storybook #210

Open
malalancette opened this issue Jun 23, 2022 · 5 comments
Open

Setting type "radios" does not work in Storybook #210

malalancette opened this issue Jun 23, 2022 · 5 comments

Comments

@malalancette
Copy link

Hi,

I've been working with Wingsuit in the past two weeks, trying to make sense of it and also of UI Patterns and UI Patterns Settings. I don't have a lot of experience with Drupal and its modules, so I read the documentation of these two modules as listed on https://wingsuit-designsystem.github.io/components/wingsuit/ and also of the Wingsuit documentation and I became really frustrated with the lack of explanations for the patterns definitions (*.wingsuit.yml files).

I had to analyse how Wingsuit work under the hood to make sense of it all. Two weeks later, I think I understand it better now. Well... I hope.

What makes me write this issue is I think there is an issue in the code for the setting type "radios".

if (
setting.getType() === 'select' ||
setting.getType() === 'radios' ||
setting.getType() === 'colorwidget'
) {
argTypes[key].type.name = 'enum';
argTypes[key].description += `<br>Option keys: ${Object.keys(setting.getOptions()).join(
', '
)}`;
argTypes[key].options = Object.keys(getStorybookControlsOptions(setting));
argTypes[key].control = {
labels: getStorybookControlsOptions(setting),
type: setting.getType() === 'radio' ? 'radio' : 'select',
};

At line 159 in the code above, I think that the first instance of the word radio, should be plural as seen at line 149. Because it is not, setting type "radios" use a <select> in Storybook instead of radio buttons.

I tested it and when I fix the word, the select box become radio buttons as you can see in the image below.

wingsuit-settings-type-radio

@christianwiedemann
Copy link
Contributor

Hi Maxim,

I know learning Wingsuit/UI Patterns is not that easy.
And I know it should be much more documented. Have you seen the slack account. Don't hessitate to ask me (or discuss) if you have any questions.

Can you describe what are your (biggest) pain points in the documentation?
Actually, an explanation Video about Wingsuit/UI Patterns would good.

(To your bug. I will check that.)

@malalancette
Copy link
Author

Hi Christian,

Thanks for the quick answer.

I looked for the slack account, but I'm not able to join. Do I need an invite or something?


Before talking about my issues with the documentation, I have to tell you, my specialty in web dev is HTML, CSS and JavaScript (not NodeJS, Webpack, etc.). I can find my way around a NodeJs and a Webpack server, but I'm not the one that set it up, because it's way outside my knowledge capabilities (when the programming logic is getting really deep, my brain shortcuts) and interests. So, if I don't have a documentation that easily explain to me what does what and what goes where, I will have difficulty to to what I have to do.

Also, I hope that I don't like I'm angry. I assure you that I'm not.


As for the documentation, it's a collegue of mine that installed Wingsuit, so for what I need to do the only part of the documentation that I use is related to pattern definition and twig file, but I also needed to add assets so the two major sections of the documentation that I used are :

With that said, I'll start with the Assets one. I needed to add Font-Awesome webfonts and Alpine.js. When I read the Assets doc, it didn't really tell me how to add the webfonts. So I searched into the Github repository and the project issues to find clues on how that works. I found one of your answers in one issue about webfonts, so I used that and it worked, but it was not anywhere in the documentation. For Alpine.js, I followed the doc for JavaScript Assets and it didn't work. So I searched the Github repository and found this file. It was exactly what I needed, but even if it was a lot alike the documentation, what was inside Drupal.behaviors.alpinejs = {} was not in the documentation. With some help from my collegue, I was able to make this works because alpinejs.vendor.js was not compiled by TypeScript (I had to add "allowJs": true option to make this works).

As for the Components documentation :

  • There is no example or explanation what to do with Global Variables. It says that I can place a YAML file, but it doesn't show what to write in it and how to use it after that.
  • It's the same thing with the Presentation page. I really don't understand what are "presentations". In a way, it looks like the Storybook MDX documentation page, but I don't know if it is related or not.
    • Also, for the "Additional Twig functions", you explain what the two functions does, but there is no example how to use them. The only way I can see how to use them is to analyse how it's used by doing a search in the Github repository.
  • Finally, the Wingsuit Patterns. A lot of my confusion was about the YAML file. Even after reading UI Patterns and UI Patterns Settings documentations, which are really, really not helpful at all, I was still not sure what to do. It took me a lot of trial and error to understand how it worked. What I can suggest to you is to document how the pattern.wingsuit.yml works, what can it do in Storybook and in Drupal (also compare the differences in Storybook vs Drupal), etc.
    • Field types :
      • UI Patterns documentation says "Field type, can be text, numeric, etc. at the moment only used for documentation purposes.". You can see how I am confused now. I know it's not a Wingsuit issue, because it's not your documentation, but to create Wingsuit I suppose you had to understand how UI Patterns works, so if it's the case, maybe you can write about it in the Wingsuit doc.
      • Also, Storybook-wise, when I analysed Wingsuit's code, I saw that you defaulted of lot of field types to a Storybook "text" control. On my local project, I tested the "numeric" type by adding a else if to use the Storybook "number" control and it worked, so I think that there is room for improvement here.
    • Setting types :
      • Again, UI Patterns Settings documentation's is thin in explanations about how to use the diffrent setting types. Since there is no documentation Wingsuit-side about "Setting types", I tried using what I saw in UI Patterns Settings doc and where it confused me is when I tried using the "radios" type, it gave me a <select> box and when I tried using the "checkboxes" type (also tried "checkbox"), but it gave me a simple text <input>. No explanation in Wingsuit doc, no explanation anywhere. After analysing Wingsuit's code, like I did for the field types, I saw that it didn't support some of Storybook's controls. The "radios" type has a bug, as explained in my first comment here. Also, "checkboxes" type is not supported like I said above. So, here too there is room for improvement and it could be mentionned in doc.
      • As I said before, to create Wingsuit I suppose you had to understand how UI Patterns Settings works. If so, maybe you could explain some of the types and how to use them. I really don't know what these types do : attributes, token, group, colorwidget and coloriswidget. I know that it's something used by Drupal, but I don't find any documentation about these types.
    • Extensions : Faker, type pattern and type object. The documentation says that "These extensions are not compatible with UI Patterns Library.". So I suppose it means that it can't be used by Drupal, isn't it? What is the purpose then? I can't see what to use them for and how to use them.
  • The last thing I can remember is that I saw no documentation about pattern_configuration. I know that it's use in some Twig files, but I saw no doc to explain what it does or how to use it.

In conclusion, what I suggest you is to add screenshots to your documentation. It will help to visualize what the documentation says.

So, that's about it.

@malalancette
Copy link
Author

Hi @christianwiedemann,

Did you see my last post?

Does that makes senses what I wrote?

I just wanted a follow up.

Thanks

@christianwiedemann
Copy link
Contributor

Hi @malalancette,

Yes it makes sense and thanks for your explanation!
So to your questions:

  • Alpine.js: The documentation is for alpine 2. So I need to update the docs.
  • Fonts: It is actualy a webpack issue not a Wingsuit issue because you can do it in serval way. Wingsuit don't support fonts right now. But I think also it should be easier to integrate it into Wingusit but I did not find a good solution where both ways work. SVG Spritemap and Fonts.
  • Yes the pattern documentation is bad and complete unclear. It is on my list. And it is a problem of the history of UI Patterns. So people who don't know Drupal it is very hard to understand it.
    Field Type: The type of field is simple not relevant. (It is part of UI Patterns)
    Faker: You can use a faker if you don't like to write some lorem ipsum dummy text for your pattern preview. In Drupal you don't need preview text.
    Pattern Setting Type: Each type is Drupal Plugin which provides a form which can used in Drupal. Here you can find the plugins
    https://git.drupalcode.org/project/ui_patterns_settings/-/tree/8.x-2.x/src/Plugin/UiPatterns/SettingType
    Pattern configuration: It is a twig function to access configuration from Wingsuit pattern file. This is helpful if you want to have differnt classes for each variant. See https://github.com/wingsuit-designsystem/wingsuit/blob/master/starter-kits/tailwind/source/default/patterns/01-atoms/button/button.wingsuit.yml

Would be great if you can come to slack than we can chat/talk directly.

@doxigo
Copy link

doxigo commented Dec 8, 2022

thanks to both of you for a detailed issue here, but I believe we got a bit off-topic here, which is a whole other issue.

I still believe the radios type should be fixed regardless, any ideas on that part @christianwiedemann ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants