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

Add ability to document functions, mixins options #386

Open
pascalduez opened this issue Mar 19, 2015 · 13 comments
Open

Add ability to document functions, mixins options #386

pascalduez opened this issue Mar 19, 2015 · 13 comments

Comments

@pascalduez
Copy link
Member

A tweet points out there is no way to document a Map as parameter currently.
That might be an interesting new feature to add.

Consider the following:

/// Sowieso
/// @param {Map} $options [()] - some options
@mixn whatever($options: ()) { ... }

There's no way to properly document the options map properties.

Some possible solutions (just to illustrate the matter):

Introduce a new annotation

/// Sowieso
/// @options $options - some options
///    @prop {Number} one [1]- option one
///    @prop {String} two ['two'] - option two
@mixn whatever($options: ()) { ... }

Pro: straightforward
Con: How to deal with indentation, nesting

Reference a Map outside the caller

/// Sowieso
/// @type {Option Map}
/// @prop {Number} one [1]- option one
/// @prop {String} two ['two'] - option two
$options: ( one: 1, two: 'two' );

/// Sowieso
/// @options $options - some options
@mixn whatever($options: ()) { ... }

Pro:
Con: Weirdo, complex

EDIT: Was already reported in #111

@valeriangalliat
Copy link
Member

We already have @property! http://sassdoc.com/annotations/#property

@pascalduez
Copy link
Member Author

We already have @Property!

But no way to apply @property to a Map as a parameter.

@valeriangalliat
Copy link
Member

Alright. Well if we make it compatible with mixin/function parameters, the syntax should remain the same, or as close as possible.

What I propose:

/// @param {Map} $options
/// @prop {String} $options.foo [default] - Description
/// @prop {Number} $options.bar [42] - Description

@pascalduez
Copy link
Member Author

Yeah, much better !
Funny, reminds me the discussions we had when implementing Maps documentation :-)

@ajmueller
Copy link

Great proposal @valeriangalliat. That would fit our needs perfectly and be very flexible for others, too. The display in the default theme could get a little bit tricky, though. As I mentioned in #111 when the default value of a map has more than a couple keys, the current display causes the table to get distorted because the Default Value column is so wide. I really like the way you currently output @prop for maps, so I'm sure you'll find a good way to do so.

@pascalduez
Copy link
Member Author

Then there is another issue as mentioned in #111

/// Sowieso
/// @param {Map} $options [()] - some options
@mixn whatever($options: ()) { 
  $defaults: (
    one: 1,
    two: 'two'
  );
}

There is no way to document an item inside an item, and I think that would be a bit problematic to implement.
That's where the Sass !default is supposed to act.

@ajmueller
Copy link

Have you all had a chance to discuss this further within the team? Let me know if you have any questions about our needs or implementation ideas.

@KittyGiraudel
Copy link
Member

Can I close since we have #111?

@ajmueller
Copy link

I believe @pascalduez opened this issue because #111 was closed.

@pascalduez
Copy link
Member Author

Yep, #111 is closed, also we are kind of speaking about 2 different features here if I'm not mistaken ?

  • Maps as params
  • Maps inside an item

@ajmueller
Copy link

I think the two are related in this instance, but are definitely separate features. The way we happen to (generally) use maps as params is to define options that are then extended by a "default" map inside a function or mixin.

@shiftgeist
Copy link

This would be super helpful! I am using maps in combination with mixins for most things like fonts size, margin, padding, etc. Currently I build in a warning if the value from the mixin parameter is not found, which is super annoying because you have to have it built first before you know what values you can use.

@yuxblank
Copy link

yuxblank commented Feb 2, 2022

Alright. Well if we make it compatible with mixin/function parameters, the syntax should remain the same, or as close as possible.

What I propose:

/// @param {Map} $options
/// @prop {String} $options.foo [default] - Description
/// @prop {Number} $options.bar [42] - Description

That is what i'm really looking for!

We use a theme map and will be cool to allow it to be documented for any mixin using it.

My proposal would be even simpler, by allowing to create custom types!

For instance:

///  @typedef Theme
/// @Theme.color {String} #fff [default] - Description
/// @Theme.size{Number} 14 - Description
/// @Theme.font.family {String} Arial- Description

So you end up with a typedef with props and nested props that you can use as parameter where you need it.
For instance:

/// @param {Theme} $theme
@mixin whatever($theme) { }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants