fix(docs-infra): render deprecated markers for CLI command options
#28111
Conversation
petebacondarwin
added
feature
petebacondarwin
added
the
target: patch
|
You can preview 7b78082 at https://pr28111-7b78082.ngbuilds.io/. |
There was a problem hiding this comment.
I think that the styling you have chosen for deprecated CLI options is a little to strong:
While we do use the red deprecated badge in the top heading for a whole page, we only use crossed through styling on the element name when referring to a deprecated within a page, with a bold Deprecated: marker followed by the deprecated message.
I think we should keep this in the same style.
| @@ -56,6 +56,7 @@ | |||
| <code class="cli-option-syntax no-auto-link">{$ renderOption(option.name, option.type, option.default, option.enum) $}</code> | |||
| </td> | |||
| <td> | |||
| {% if option.deprecated !== undefined and option.deprecated !== false %}<label class="raised cli-deprecated">deprecated</label> {% if option.deprecated !== true %} {$ option.deprecated | marked $} {% endif %}{% endif %} | |||
There was a problem hiding this comment.
In the CLI code base the deprecated value is:
/**
* Deprecation. If this flag is not false a warning will be shown on the console. Either `true`
* or a string to show the user as a notice.
*/
deprecated?: boolean | string;So I think it is actually enough to check {% if option.deprecated %}, right? If it is undefined or false then this if will fail. Or are we concerned about the empty string: "" - @hansl ?
|
About the style, I've been inspired by the read-only label . But, I agree that it could be shrill. I'll change this. |
|
So, with the actual system of markdown renderer, we cannot have 'Deprecated' and a string on the same line. Have 'Deprecated' and 'its explain' on two separate lines is ambiguous. Either we have a boolean deprecated: trueand it'll display Deprecated or we have a string and we have to set deprecated: "**Deprecated:** lorem ipsum"and it'll display Deprecated: lorem ipsum It's ok for you ? |
|
Check out this template: angular/aio/tools/transforms/templates/api/lib/memberHelpers.html Lines 51 to 54 in 18b6d58 |
|
You can preview ad8cdcc at https://pr28111-ad8cdcc.ngbuilds.io/. |
|
You can preview 71f1269 at https://pr28111-71f1269.ngbuilds.io/. |
petebacondarwin
added
the
action: merge
| @@ -7,4 +7,3 @@ | |||
| .cli-option-syntax { | |||
| white-space: pre; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
😭
UPDATE: Oh, it was an extra line 🤗
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
fixes #27563
fixes #27423
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Show a deprecated label near option cli deprecated
What is the current behavior?
Issue Number: #27563 #27423
No deprecated information near cli options that were deprecated. The information exists however in build.json file.
{ "name": "vendorSourceMap", "description": "Resolve vendor packages sourcemaps.", "type": "boolean", "default": false, "required": false, "aliases": [], "hidden": false, "deprecated": true },What is the new behavior?
We have a deprecated label and we can add a message below whether we change
trueby a text:{ "name": "vendorSourceMap", "description": "Resolve vendor packages sourcemaps.", "type": "boolean", "default": false, "required": false, "aliases": [], "hidden": false, "deprecated": "lorem ipsum ..." },Does this PR introduce a breaking change?
Other information