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 #25977

Closed
MartijnCuppens opened this issue Mar 30, 2018 · 9 comments
Closed

Mixins documentation #25977

MartijnCuppens opened this issue Mar 30, 2018 · 9 comments

Comments

@MartijnCuppens
Copy link
Member

Not every mixin is documented in the Bootstrap docs. Maybe we should add a dedicated place for mixins in the docs as @kevdotbadger suggested here.

@Varunram
Copy link
Contributor

Yeah, most of the mixins are distributed over multiple parts, would be useful to consolidate them in one place.

@MartijnCuppens
Copy link
Member Author

MartijnCuppens commented May 1, 2018

I'm thinking about creating a jekyll plugin which collects all mixin-names from the .scss-files and prints them on a documentation page. Maybe add the comments above the mixin as a description below each name? And a link which opens a modal with the mixin code? In the end it would look somewhat like this:

image

Thoughts?

@hunkjazz
Copy link
Contributor

hunkjazz commented Jan 21, 2019

I coudn't found this issue before, so I did a documentation for functions, mixins and variables, using SassDoc.

@MartijnCuppens Do you think it is worth adding a reference link for my documentation, while you build the official one?

Bootstrap SassDoc

@MartijnCuppens
Copy link
Member Author

@JLammeer, we like to have control over the documentation ourselves to prevent incorrect documentation and to keep our code and docs in sync. But you did a nice job with this documentation (I noticed the since versions are not always correct, but that's just a detail).

We were working on #26500 (which is currently on hold until v5). We might have a look at the way you handled things when we continue with it.

@hunkjazz
Copy link
Contributor

@MartijnCuppens Don't worry. I took this risk, from the beginning. 👍

@thetiby
Copy link

thetiby commented May 10, 2019

@MartijnCuppens
There is an error in the official documentation (and, thus, in the generated one with SassDoc - cc @JLammeer), for the media query mixins:
image
I did this example as I didn't want to rely on my local build, that could've been flawed. Used latest v4.3.1 source files.

Here's a copy of the input and output, as I wasn't able to share the online test case:

Input:

@import "_functions";
@import "_variables";
@import "_breakpoints";

@include media-breakpoint-between(md, xl) {
    body:before {
        /* Per documentation...
        https://getbootstrap.com/docs/4.3/layout/overview/#z-index
        */
        content: "...this should be @media (min-width: 768px) and (max-width: 1199.98px) ";
    }
}

/* Generated documentation at
    https://jlammeer.github.io/bootstrap-sassdoc/#breakpoint-mixin-media-breakpoint-down
    According to it...
*/
@include media-breakpoint-down(sm, (xs: 0, sm: 576px, md: 768px, lg: 992px, xl: 1200px)){
  .your-custom-class:before {
    content: '...this hould be @media (max-width: 576.98px) {';
  }
}
/*
The documentation in the source comments and the documentation available online don't match (most likely the documentation is wrong)

The source says:
// Media of at most the maximum breakpoint width. No query for the largest breakpoint.

The documentation says:
// Example: Style from medium breakpoint and down

it is correct, but misleading, thus even the people doing the documentation got confused (see first example)

more accurate would be:
// Example: Style from large breakpoint and down, exclusive
*/
@include media-breakpoint-down(md) {
  .custom-class {
    display: block;
  }
}

Output:

@media (min-width: 768px) {
  /* line 6, stdin */
  body:before {
    /* Per documentation...
        https://getbootstrap.com/docs/4.3/layout/overview/#z-index
        */
    content: "...this should be @media (min-width: 768px) and (max-width: 1199.98px) ";
  }
}

/* Generated documentation at
    https://jlammeer.github.io/bootstrap-sassdoc/#breakpoint-mixin-media-breakpoint-down
    According to it...
*/
@media (max-width: 767.98px) {
  /* line 19, stdin */
  .your-custom-class:before {
    content: '...this hould be @media (max-width: 576.98px) {';
  }
}

/*
The documentation in the source comments and the documentation available online don't match (most likely the documentation is wrong)

The source says:
// Media of at most the maximum breakpoint width. No query for the largest breakpoint.

The documentation says:
// Example: Style from medium breakpoint and down

it is correct, but misleading, thus even the people doing the documentation got confused (see first example)

more accurate would be:
// Example: Style from large breakpoint and down, exclusive
*/
@media (max-width: 991.98px) {
  /* line 38, stdin */
  .custom-class {
    display: block;
  }
}

@hunkjazz
Copy link
Contributor

hunkjazz commented May 10, 2019

In fact, this would be more accurate:

Style from large breakpoint and down, exclusive

And I would accept a PR for this. Although, I should note that this doc isn't official nor up to date. Maybe I'll have it updated in the next month.

@ffoodd ffoodd added the has-pr label May 29, 2020
@XhmikosR
Copy link
Member

@MartijnCuppens do we keep this open or the current solution covers it?

@mdo
Copy link
Member

mdo commented Jan 10, 2021

Closing in favor of something like #32747.

@mdo mdo closed this as completed Jan 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants