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

excludeDefaultStyles documentation is confusing #1088

Closed
DanielaValero opened this issue May 16, 2017 · 3 comments
Closed

excludeDefaultStyles documentation is confusing #1088

DanielaValero opened this issue May 16, 2017 · 3 comments

Comments

@DanielaValero
Copy link
Contributor

DanielaValero commented May 16, 2017

Hello,

To my understanding, the default styles are contained in: styleguide-app.

If this is correct, and the new option: excludeDefaultStyles is set to true, then the styleguide-app.css should not be delivered to the app.

However, at the moment, it is working in the opposite way. If excludeDefaultStyles is set to true, then the app styles are sent to the app.

The issue now, is that its default value is false, so, when excludeDefaultStyles === false, then the styles are not being sent to the app, which causes that the apps of the users now don't get the styles.

To my understanding, we should do any of the next options:

  • Rename the option to: includeDefaultStyles and set it to true by default, to avoid issues in the users
  • Fix the behaviour that is implemented at the moment. So: excludeDefaultStyles === false send the styles. And excludeDefaultStyles === true send the styles. And define it as false by default
  • Improve the documentation of the new options, so it is clear what it does, and what the default styles are.
@varya
Copy link
Contributor

varya commented May 16, 2017

@junaidrsd

@junaidrsd
Copy link
Contributor

@DanielaValero styleguide-app.css contains other definitions inside it which is important in both cases for example font styles and custom colors. If excludeDefaultStyle option is set then styleguide does not compile default styles in styleguide-app.css. I’ll make sure both options are working. I think it is better to have false as a default value that is why I set excludeDefaultStyle to true so that user wont effect by this change.

@DanielaValero
Copy link
Contributor Author

Hi @junaidrsd

I did a new check. I set in my config explicitlyexcludeDefaultStyles: false.

Then the styles are rendered, but not on page load. But somehow, it does not behave in all the cases consistently.

It might be that what happens is that the default value is not being taken properly by the implementation.

However, about the loading consistently or inconsistently, I will try to get more details on the steps to reproduce, and let you know if/when I get more info.

@varya varya closed this as completed in 3afedfd Jun 1, 2017
varya added a commit that referenced this issue Jun 1, 2017
…lean-install

Fix #1088 customstyles missing on clean install
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