ember-learn / ember-cli-addon-docs

Easy, beautiful docs for your OSS Ember addons
https://ember-learn.github.io/ember-cli-addon-docs
MIT License
176 stars 143 forks source link

Default styles were removed with the Tailwind v1 upgrade. Add default styles for a good first time experience? #453

Open NullVoxPopuli opened 4 years ago

NullVoxPopuli commented 4 years ago

Update 2 (after this comment: https://github.com/ember-learn/ember-cli-addon-docs/issues/453#issuecomment-583858157)

"Default styles" were removed in Tailwind V1 (i.e: Tailwind's reset stylesheet sets all elements to look the exact same), resulting in:

The solution:


Update 1 (after checking previous versions):

Looks like this release broke the default styles: https://github.com/ember-learn/ember-cli-addon-docs/releases/tag/v0.6.16


Original Issue Report

Info / Debug Session / me being very confused: https://discordapp.com/channels/480462759797063690/484421898210377729/675606201077202954

Reproduction: https://github.com/NullVoxPopuli/AddonDocsBrokenCssReproduction

it looks like all elements have been stripped of all styles.

image

Steps in readme copied here:

Reproduction

npx ember-cli addon ec-addon-docs-repro --yarn
cd ec-addon-docs-repro
yarn

https://ember-learn.github.io/ember-cli-addon-docs/docs/quickstart

ember install ember-cli-addon-docs
# choose ESDoc or install below
ember install ember-cli-addon-docs-esdoc

following the rest of the quickstart, ignoring template linting errors.

during the ember generate docs-page usage step, fix the router:

Router.map(function() {
  docsRoute(this, function() { /* Your docs routes go here */ });
  this.route('usage');
});

should be

Router.map(function() {
  docsRoute(this, function() { 
    /* Your docs routes go here */ 
    this.route('usage');
  });
});

Because Octane no longer has the application-template-wrapper, the tests/dummy/app/templates/application.hbs must be wrapped with

<div class='ember-view'>
  <!-- the original content of this file -->
  {{docs-header}}

  {{outlet}}

  {{docs-keyboard-shortcuts}}
</div>

Now, to add a header and list: in tests/dummy/app/templates/index.hbs, replace

<div style="max-width: 40rem; margin: 0 auto; padding: 0 1.5rem">
  {{#docs-demo as |demo|}}
    {{#demo.example name="my-demo.hbs"}}
<!-- (yes, I read up on DocsDemo, 
https://ember-learn.github.io/ember-cli-addon-docs/docs/api/components/docs-demo, 
but there is next to no information on it, and it doesn't seem to be relevant for just 
having non-demo / static content? idk.) -->
      <p>Make sure to read up on the DocsDemo component before building out this page.</p>
    {{/demo.example}}
  {{/docs-demo}}
</div>

with


<div class="docs-container docs-md docs-my-16">
  <section class="docs-section docs-max-w-md docs-mx-auto docs-pb-8">
    <h2>h2</h2>

    <p>
      paragraph text
      <ul>
        <li>item 1</li>
        <li>item 2</li>
        <li>item 3</li>
      </ul>
    </p>

    <div class="docs-my-5 docs-text-right">
      {{#docs-link "docs"}}
        <button
          type="button"
          class="docs-bg-grey-darkest docs-text-white docs-py-2 docs-px-4 docs-no-underline docs-rounded hover:docs-bg-black"
        >
          Get started &rarr;
        </button>
      {{/docs-link}}
    </div>
  </section>
</div>

where did I get this structure / classes from? here: https://github.com/alexdiliberto/ember-transformicons/blob/master/tests/dummy/app/templates/index.hbs#L4 and the h2 looks correct https://alexdiliberto.com/ember-transformicons/

NullVoxPopuli commented 4 years ago

Looks like this is fixed by pinning ember-cli-addon-docs to version 0.6.14, from the latest 0.6.16

NullVoxPopuli commented 4 years ago

if 0.6.16 was intended to be a breaking release, that's fine. but should the release notes be updated to suggest that nothing is allowed to be classless?

NullVoxPopuli commented 4 years ago

I confirm that <= 0.6.15 gets me the style behavior I was expecting: image

NullVoxPopuli commented 4 years ago

Though, would this mean that markdown transformations are also broken?

image

RobbieTheWagner commented 4 years ago

@NullVoxPopuli I am not quite sure which styles you are saying are broken? Are you referring to headings not having default font sizes?

NullVoxPopuli commented 4 years ago

yeah, lack of sizes, bolding, padding, margin, etc.

NullVoxPopuli commented 4 years ago

Kind of most surprising is the markdown not having any styles.

RobbieTheWagner commented 4 years ago

@NullVoxPopuli I'm not sure I am seeing all that completely unstyled. I know Tailwind removed default heading sizes in 1.x, so we would have to make sure we are shipping those. Perhaps the same is true for whatever other styles you are seeing not included?

NullVoxPopuli commented 4 years ago

So, how would I style markdown? Do I now need to write all my own css, cause with current tailwind, all markdown is the same unstyled text?

RobbieTheWagner commented 4 years ago

@NullVoxPopuli I'm not seeing everything unstyled like you are saying. There are styles here https://shipshapecode.github.io/ember-math-helpers/docs/configuration and that is using the latest ember-cli-addon-docs.

You'll notice that the heading is not styled though. Tailwind v1 removed default heading styles, so I believe addon docs should take an opinionated stance and add back equivalents for the pre-1.x tailwind heading styles. https://tailwindcss.com/docs/upgrading-to-v1/#6-explicitly-style-any-headings

It also appears that lists are now unstyled in tailwind v1. https://tailwindcss.com/docs/upgrading-to-v1/#7-explicitly-style-any-lists-that-should-have-bullets-numbers

If these are the types of issues you are experiencing, I would be open to adding back opinionated defaults for these.

Thoughts @samselikoff @josemarluedke?

NullVoxPopuli commented 4 years ago

If these are the types of issues you are experiencing, I would be open to adding back opinionated defaults for these.

Yes, that's what I was trying to say the whole time ;) (and is apparent in the screenshots? Maybe I needed highlighting or something)

Like, HTML having style stripped/reset is a stance you can take, but markdown doesn't have the ability to add classes, right? Also, what's the point of markdown if there is no difference between a list item and a header, ya know?

RobbieTheWagner commented 4 years ago

Yes, that's what I was trying to say the whole time ;) (and is apparent in the screenshots? Maybe I needed highlighting or something)

It sounded like you were saying everything was completely unstyled, and that markdown included no styles.

Kind of most surprising is the markdown not having any styles.

I've been trying to confirm if the issues you were seeing were from Tailwind v1 changing things or if your entire setup was broken and truly had zero styles. It sounds like Tailwind v1 issues so far to me, so I would be in favor of adding back the old Tailwind pre v1 styles for things like headings, lists, etc.

Would you be interested in opening a PR for this @NullVoxPopuli?

NullVoxPopuli commented 4 years ago

It sounded like you were saying everything was completely unstyled, and that markdown included no styles.

i could see how it could seem that way 🤔

Would you be interested in opening a PR for this @NullVoxPopuli?

would be better to define defaults? and have opt out?

RobbieTheWagner commented 4 years ago

I don't think we really need to have an opt out. This tool should be opinionated and generate formatted documentation. If you want to deviate from that, it can be done manually with CSS. Although, I suppose an opt out wouldn't hurt anything.

NullVoxPopuli commented 4 years ago

Based on what I read for the reason for namespacing the addon-docs' styles, is that people wanted to use their own styles without having to override addon-docs'. If we didn't add an opt out, providing a set of defaults would then frustrate the people who want to customize everything?

RobbieTheWagner commented 4 years ago

Based on what I read for the reason for namespacing the addon-docs' styles, is that people wanted to use their own styles without having to override addon-docs'. If we didn't add an opt out, providing a set of defaults would then frustrate the people who want to customize everything?

I would still namespace these styles. Unsure exactly how to accomplish it, but we should continue the docs- for styles I think.

I'm definitely open to suggestions here.

NullVoxPopuli commented 4 years ago

well, does markdown generate anything with classes? or is it just the HTML?

RobbieTheWagner commented 4 years ago

I think it would have to be a custom marked renderer or something. Unsure how things work now, but we could certainly make it work. https://marked.js.org/#/USING_ADVANCED.md#options