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

Angular Feedback #2645

Closed
amcdnl opened this issue Jan 4, 2018 · 12 comments
Closed

Angular Feedback #2645

amcdnl opened this issue Jan 4, 2018 · 12 comments

Comments

@amcdnl
Copy link
Member

amcdnl commented Jan 4, 2018

First off, this is awesome guys! I know first hand what a challenge Angular can be to build this type of stuff for! I gave it a whirl today and here is my general 'end-user' feedback.

  • Add SASS and LESS support by default when you scaffold
  • Make scaffolded the stories and config TypeScript not JS
  • Every example should show importing a NgModule
  • Add option for defining a template in the story like you did with Vue
  • The ability to specify a custom tsconfig for storybook
  • The angular cli can include global css in the .angular-cli.json that lots of ppl use, it should be picked up here too
  • When scaffolding put /stories under /src so TypeScript will pick it up by default in a cli project

A few notes on some of those items:

//cc @igor-dv

@ndelangen
Copy link
Member

//cc @alterx @ralzinov

@alterx
Copy link
Member

alterx commented Jan 4, 2018

Lovely feedback @amcdnl!

We do use dynamic components currently so adding the template support shouldn't be very-
difficult. These examples are amazing (the inputs/outputs parsing code will help a lot)!

@Quramy
Copy link
Contributor

Quramy commented Jan 6, 2018

The angular cli can include global css in the .angular-cli.json that lots of ppl use, it should be picked up here too

I think it's nice !

Now, @storybook/angular does not depend on @angular/cli. I understand that it makes sense for developers not using @angular/cli. However, almost all Angular developers create their project from ng new, so they(including me) want that start-storybook bundles module configured with their .angular-cli.json.

I think storybook should provide a helper function to configure webpack based on .angular-cli.json, something like:

/* .storybook/webpack.config.js */

import { webpackConfigHelperForNgCli } from '@storybook/angular';

module.exports = (env) => {
  return webpackConfigHelperForNgCli(); // configures .ts , HTML, and style Rules 
};

@alterx
Copy link
Member

alterx commented Jan 7, 2018

Hey @amcdnl , I have a couple of questions for you:

  • I think the /stories folder is already inside the /src. Am I missing something here? 🤔
  • Could you please elaborate on Every example should show importing a NgModule ? For basic components it's possible to not import any modules in Storybook.

Just trying to get a more clear idea of the stuff we want to focus on :D

@Quramy
Copy link
Contributor

Quramy commented Jan 7, 2018

@alterx

I think the /stories folder is already inside the /src. Am I missing something here? 🤔

getstorybook generator creates the stories dir under not <project-root>/src but <project-root> . I'll fix it in PR #2672, would you review this?

@amcdnl
Copy link
Member Author

amcdnl commented Jan 8, 2018

@alterx -

  1. @Quramy explains the issue.
  2. I don't think showing just a component without ngModule imports is a good representative of a true setup since almost every demo will need to include a module.

@alterx
Copy link
Member

alterx commented Jan 8, 2018

@amcdnl

  1. There's a PR that implements this! :D
  2. Ok, so here you're suggesting updating the docs. Gotcha :)

@alterx
Copy link
Member

alterx commented Jan 10, 2018

Progress on these items:

@igor-dv
Copy link
Member

igor-dv commented Jan 21, 2018

Are we going to do these 2 items? What is the meaning of them?

@amcdnl
Copy link
Member Author

amcdnl commented Jan 21, 2018

They are more documentation based items.

@alterx
Copy link
Member

alterx commented Jan 21, 2018

@amcdnl , @igor-dv
For item 1 our stories are already written in .ts files.
Is there a reason to write the config in ts? I mean, the .storybook folder is inside the top directory of the project, not inside the src folder and other configurations such as protractor.conf.js and karma.conf.js are written in JS.

For item 2 we should update the examples and I'm thinking we should also remove the FormsModule from the source (the BrowserModule should stay there since we're bootstrapping an Angular app and we need it)

@stale
Copy link

stale bot commented Mar 7, 2018

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 60 days. Thanks!

@stale stale bot added the inactive label Mar 7, 2018
@amcdnl amcdnl closed this as completed Mar 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants