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

Mixins documentation page #26500

Closed
wants to merge 7 commits into from

Conversation

MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented May 12, 2018

Fixes #25977.

All mixins are generated by the mixins_documentation jekyll plugin under the _plugins folder. This plugins executes the npm run mixins-documentation --silent command. The --silent parameter prevents the filename to be outputted.

npm run mixins-documentation --silent runs build/mixins-documentation.js. This script reads all the .scss files in the scss/mixins folder and returns a list of the mixin names, the file location and the mixin itself in code block.

It's also possible to add a description to a mixin. Therefore you can add a comment block just above the mixin (no blank lines between the mixin and the comment block). Markdown is supported in these blocks. Stylelint comments are ignored.

Demo: https://deploy-preview-26500--twbs-bootstrap4.netlify.com/docs/4.2/extend/mixins/

TODO:

  • fix stylelint comments detection
  • switch to shell.js's find
  • link to the actual mixin file
  • automatically convert links
  • move Mixins page under Extend nav
  • enable stylelint rule to check for newlines between mixins, and no newlines before mixins when there's a comment
  • maybe sort mixins alphabetically; currently they are sorted by their filename and then alphabetically
  • use smaller headers for mixins in the same file so that ToC detects them properly
  • decouple task from Jekyll maybe (remove the Jekyll plugin)
  • show a callout for deprecated mixins
  • fix link to file line bug
  • Check if Mixins documentation #25977 (comment) could be useful to build the docs

@XhmikosR
Copy link
Member

XhmikosR commented Oct 20, 2018

Wow I totally missed this patch :/

From a quick look I like it. But I still see stylelint comments in the output.

Also, I wouldn't enforce Markdown in the comments. We could just use a lib maybe?

EDIT: and I think it would be better to sort them alphabetically.

@MartijnCuppens
Copy link
Member Author

@XhmikosR, should'nt we leave the stylelint comments? And do you have any suggestions for a lib we can use to transform the links in clickable links?

@XhmikosR
Copy link
Member

XhmikosR commented Oct 21, 2018

I think the Stylelint comments don't offer any help here where we just want the code and we shouldn't assume everyone is using Stylelint.

For libs there are plenty, not sure which one to use, maybe check them out with https://packagephobia.now.sh/

and probably even more :)

EDIT:
Actually, scratch my comment about Markdown in comments... regardless if we use a lib we still need the comments to be written in Markdown :P

So basically the only gain in using a lib is so that we don't reinvent the wheel that's all. Although your solution seems to already work good enough, I'm just trying to future-proof this.

@XhmikosR XhmikosR force-pushed the mixin-documentation branch 2 times, most recently from ebdf6f1 to 848f7cd Compare October 21, 2018 11:35
@XhmikosR
Copy link
Member

I rebased this and squashed it. Not sure about the plugin which runs the npm script though. I don't like mixing the two environments a lot.

@XhmikosR XhmikosR force-pushed the mixin-documentation branch from 848f7cd to aaa66ea Compare October 21, 2018 11:45
@XhmikosR

This comment has been minimized.

@@ -3,9 +3,11 @@
@mixin float-left {
float: left !important;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW this reminds me, we should have stylelint check for this newline.

@XhmikosR XhmikosR force-pushed the mixin-documentation branch 4 times, most recently from 0386b1d to a11d1d3 Compare October 23, 2018 14:35
@MartijnCuppens

This comment has been minimized.

@XhmikosR

This comment has been minimized.

@MartijnCuppens

This comment has been minimized.

@MartijnCuppens

This comment has been minimized.

@XhmikosR

This comment has been minimized.

@XhmikosR
Copy link
Member

XhmikosR commented Oct 29, 2018

@MartijnCuppens: I pushed a couple of patches, feel free to squash everything or we do it when we merge.

I also added a couple more TODOs.

@MartijnCuppens
Copy link
Member Author

@XhmikosR,

enable stylelint rule to check for newlines between mixins, and no newlines before mixins when there's a comment

Don't think there is a stylelint rule for this at the moment

maybe sort mixins alphabetically; currently they are sorted by their filename

I tried to store the output in an array, but the streams happen async, so I cant just sort all mixins after the scss.forEach((file) => {...}). I don't know the appropriate way to handle this?

use smaller headers for mixins in the same file so that ToC detects them properly

Can you give an example of what you mean?

decouple task from Jekyll maybe (remove the Jekyll plugin)

Any other suggestions here?

MartijnCuppens and others added 7 commits December 30, 2018 18:18
…under the `_plugins` folder. This plugins executes the `npm run mixins-documentation --silent` command. The `--silent` parameter prevents the filename to be outputted.

`npm run mixins-documentation --silent` runs `build/mixins-documentation.js`. This script reads all the `.scss` files in the `scss/mixins` folder and returns a list of the mixin names, the file location and the mixin itself in code block.

It's also possible to add a description to a mixin. Therefore you can add a comment block just above the mixin (no blank lines between the mixin and the comment block). Markdown is supported in these blocks. Stylelint comments are ignored.
Not so robust, for example fails if the comment ends with `,` after the link, but seems to cover our cases.
@XhmikosR XhmikosR force-pushed the mixin-documentation branch from 378b702 to 69bab91 Compare December 30, 2018 16:18
@XhmikosR
Copy link
Member

The line counter seems wrong, most noticeable in mixins that are not the only ones in a file.

@MartijnCuppens MartijnCuppens added v5 and removed v4 labels Jan 10, 2019
@MartijnCuppens
Copy link
Member Author

@mdo, we're going to postpone this to v5 since we'll need to fix some issues and add more documentation. I'm probably going to create a new branch under twbs to make it easier to work on this.

@mdo
Copy link
Member

mdo commented Jun 16, 2020

Thoughts on the status of this @MartijnCuppens?

@mdo mdo changed the base branch from master to main June 16, 2020 20:17
@MartijnCuppens
Copy link
Member Author

I think we now better use scss-docs we recently introduced to document code fragments

Copy link

@frank101594 frank101594 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file:///private/var/mobile/Containers/Shared/AppGroup/CF621F42-9601-44F7-8BFF-C5990641C86D/File%20Provider%20Storage/88219381/1IzQCUzzb1E84lB6JR47zMDg5wAFb9ZGa/memories_history.html

@MartijnCuppens MartijnCuppens deleted the mixin-documentation branch June 26, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mixins documentation
5 participants