docs: add api spec #241
docs: add api spec #241
Conversation
src/object-properties.js
Outdated
| index: -1, | ||
| /** | ||
| * Color defined by rgb, argb or html |
There was a problem hiding this comment.
Maybe others work as well, but we generally want this to be a hex color
src/object-properties.js
Outdated
| * @type {object} | ||
| */ | ||
| border: { | ||
| /** | ||
| * Shop top border |
|
Which of these properties are optional? I ran into the problem for the action button that if you want to change like one property for the styling, you override all the others and there are no sanity checks for that in the code, so you have to define the defaults again... What is the case for org chart? Maybe no need to change it, but it is a nicer experience for mashup developers |
src/object-properties.js
Outdated
|
|
||
| /** | ||
| * @typedef {object} FontColor | ||
| * @property {('auto'|'colorPicker'|'byExpression')=} colorType - How the font color is defined, defaults to auto |
There was a problem hiding this comment.
For properties with primitive values and where the value is optional. You can specify a default with something like this I believe:
@property {('auto'|'colorPicker'|'byExpression')} [colorType='auto']
That would should generate a default value in the spec as well so no need to type the default value in the property description.
For the case with objects as props I'll have to get back to you on how to best define that.
There was a problem hiding this comment.
That is really useful. Would it still be considered a default value even if there is no real default value on the properties per se, but handled with a fallback value in the styling logic? Asking since I wonder if this would work for the table as well
There was a problem hiding this comment.
I would say the default value is there to indict to the user how the behaviour will be, given that they do not provide a value. So it should not matter how it's implemented. As long as the default value results in the same behaviour as when they do set an explicit value.
There was a problem hiding this comment.
Great then I'll add it to my PR as well
api-specifications/properties.json
Outdated
| "type": "boolean" | ||
| }, | ||
| "fullBorder": { | ||
| "description": "fullBorder - Set to true to show full border, default is false", |
There was a problem hiding this comment.
The description looks odd with the property name there as well. Try changing the jsdoc definition to:
@property {boolean} [fullBorder=false] Set to true to show full border
|
FYI you can run the dev-portal locally the test how the API spec appears in the dev-portal. To get a better sense if it looks alright or not. |
|
Union types don't seem to work on base-level properties :/
Otherwise I think it's looking pretty good once I remove the property names from the description |
|
or no, because properties should be one whole object, which would probably solve the above problem - might just be that I have some issue in my scriptappy setup |
api-specifications/properties.json
Outdated
| { | ||
| "kind": "literal", | ||
| "value": "'regular'" | ||
| }, | ||
| { | ||
| "kind": "literal", | ||
| "value": "'scroll'" | ||
| }, | ||
| { | ||
| "kind": "literal", | ||
| "value": "'free'" | ||
| } |
There was a problem hiding this comment.
These are not the correct options right? I think there is free and expandAll mode. We have talked about different modes, but never implemented any
| @@ -33,6 +33,7 @@ | |||
| "sense": "nebula sense --ext ./src/extension/ext-raw.js --meta ./src/extension/meta.json", | |||
| "lint": "eslint src", | |||
| "start": "nebula serve --type sn-org-chart", | |||
| "spec": "scriptappy-from-jsdoc -c ./spec-configs/props.conf.js", | |||
There was a problem hiding this comment.
Does this need to run automatically? E.g. when version is bumped?
There was a problem hiding this comment.
I'll do that in a separate PR
src/object-properties.js
Outdated
| /** | ||
| * @typedef {object} FontColor | ||
| * @property {('auto'|'colorPicker'|'byExpression')=} [colorType='auto'] How the font color is defined | ||
| * @property {PaletteColor=} color - Color defined by index or hex code, needed if colorType is colorPicker | ||
| * @property {ColorExpression=} colorExpression - Color defined by expression, needed if colorType is byExpression | ||
| */ | ||
|
|
||
| /** | ||
| * @typedef {object} Background | ||
| * @property {('auto'|'colorPicker'|'byExpression')=} [colorType='colorPicker'] How the font color is defined | ||
| * @property {PaletteColor=} color - Color defined by index or hex code, needed if colorType is colorPicker | ||
| * @property {ColorExpression=} colorExpression - Color defined by expression, needed if colorType is byExpression | ||
| */ |
There was a problem hiding this comment.
These two are identical, could be one generic color type
There was a problem hiding this comment.
They are basically the same, but they have different default values for colorType
There was a problem hiding this comment.
you're right, my bad! It still says font on line 88 though
I think this is my first time using scriptappy but it went ok