Docs cleanup

[dvdherron]

Description

A lot of changes were made for the upcoming release. This PR tries to make sure that

• All changes and additions have been documented and accounted for in the changelog(s)
• Links are up to date and point to the correct places

Steps to test/reproduce

I've been scanning related closed PRs to get an overview of what has changed.

These are the ones where the bulk of the changes were made:

1. Modules Core
2. Modules Init
3. Type
4. Color
5. Scale
6. Layout
7. Animate

Please explain how to best reproduce the issue and/or test the changes locally (including the pages/URLs/views/states to review).

[dvdherron]

I want Accoutrement on Sass Modules

[dvdherron]

@mirisuzanne @stacyk This is ready to start reviewing.

Some questions and known issues:

• Do we want to remove all Eyeglass references? I think all mentions of it in the modules are removed, there are just references in the README and the Changelog.
• Should the "Pre-Registered Functions" section show in the public docs? I left a question above about why that might not be working.
• In the map-compile-with example (under tokens/compile) I'm not sure why the compiled result is returning null

If you you notice anything else missing or or weird just let me know

[mirisuzanne]

@dvdherron

Do we want to remove all Eyeglass references? I think all mentions of it in the modules are removed, there are just references in the README and the Changelog.

I don't know. We still support eyeglass in the npm package. It just seemed weird to highlight it, as one of many possible package managers/bundlers? Maybe there's some way to just note in one place that tools like eyeglass or webpack will impact the import path? @jgerigmeyer may have better ideas how/where to note that?

Should the "Pre-Registered Functions" section show in the public docs? I left a question above about why that might not be working.

I… can't figure this out. Nested folders shouldn't make any difference here, but yes - when I move the files, they suddenly appear in the docs. @jgerigmeyer any idea why that might be? We can move all the files, I'd just like to understand what's going on.

In the map-compile-with example (under tokens/compile) I'm not sure why the compiled result is returning null

• The example runs a map through the color() function.
• The color function looks for each of those color names in the tools.$colors map
• The colors are not defined in tools.$colors, only in the example map

There are two possible solutions:

• change $brand-colors to be tools.$colors, so that the color function can find them
• pass in $brand-colors as a $source argument, the same way we do in compile-colors

Probably the first option is simplest?

If you you notice anything else missing or or weird just let me know

I'll do a full review here in a bit…

[jgerigmeyer]

Do we want to remove all Eyeglass references? I think all mentions of it in the modules are removed, there are just references in the README and the Changelog.

I don't know. We still support eyeglass in the npm package. It just seemed weird to highlight it, as one of many possible package managers/bundlers? Maybe there's some way to just note in one place that tools like eyeglass or webpack will impact the import path? @jgerigmeyer may have better ideas how/where to note that?

I don't have a strong feeling about this. It seems like what's there isn't hurting anything (although the Eyeglass example in the README seems identical to the non-Eyeglass example), but I'm also fine removing the specifics since Eyeglass/Webpack users would likely know how to import npm packages. 🤷

Should the "Pre-Registered Functions" section show in the public docs? I left a question above about why that might not be working.

I… can't figure this out. Nested folders shouldn't make any difference here, but yes - when I move the files, they suddenly appear in the docs. @jgerigmeyer any idea why that might be? We can move all the files, I'd just like to understand what's going on.

Fixed in 2f0e2d1 (see https://medium.com/@jakubsynowiec/you-should-always-quote-your-globs-in-npm-scripts-621887a2a784).

[stacyk]

@dvdherron Thanks for compiling all this info together. I left a few optional comments so I am going to mark this as approved on my end even though I know there is still some work that may need to be done based on the conversations taking place.

[stacyk]

Do we want to address the spacing issues that we are seeing on projects that are using this init-free version of Accoutrement?

[dvdherron]

I think it could be worth it to add a note mentioning the specific layout issues we've noticed, and/or a general note saying "this might break layouts" . . . I'll try to gather what we've noticed so far and add that in.

[dvdherron]

@dvdherron

Do we want to remove all Eyeglass references? I think all mentions of it in the modules are removed, there are just references in the README and the Changelog.

I don't know. We still support eyeglass in the npm package. It just seemed weird to highlight it, as one of many possible package managers/bundlers? Maybe there's some way to just note in one place that tools like eyeglass or webpack will impact the import path? @jgerigmeyer may have better ideas how/where to note that?

Yeah, like Jonny said I guess having at least one mention of it doesn't hurt. Since we removed on all of the modules I just wanted to double check that we hadn't merely overlooked the remaining references.

Should the "Pre-Registered Functions" section show in the public docs? I left a question above about why that might not be working.

I… can't figure this out. Nested folders shouldn't make any difference here, but yes - when I move the files, they suddenly appear in the docs. @jgerigmeyer any idea why that might be? We can move all the files, I'd just like to understand what's going on.

Jonny's fix got these to show. I'll double check to make sure everything there is looking okay.

In the map-compile-with example (under tokens/compile) I'm not sure why the compiled result is returning null

* The example runs a map through the `color()` function.

* The color function looks for each of those color names in the `tools.$colors` map

* The colors are not defined in `tools.$colors`, only in the example map

There are two possible solutions:

* change `$brand-colors` to be `tools.$colors`, so that the color function can find them

* pass in `$brand-colors` as a `$source` argument, the same way we do in `compile-colors`

Probably the first option is simplest?

Makes sense. Cleaned this up and it's compiling correctly now.

If you you notice anything else missing or or weird just let me know

I'll do a full review here in a bit…

👍🏽

[mirisuzanne]

Wow, this is excellent. I left a few comments on small cleanup, but overall very thorough.

[dvdherron]

@mirisuzanne I think these changes cover all of your review comments.

[mirisuzanne]

looks great!

[jgerigmeyer]

Two questions, but neither need to block this. 👍