Skip to content
This repository has been archived by the owner on Feb 28, 2023. It is now read-only.

feat(Ruby): add intro sentence to summary with friendly api name #373

Merged
merged 36 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
99f2b83
feat(ruby): add friendly api name to page title
dansaadati Apr 1, 2022
914c36b
feat(ruby): add friendly api name to page title
dansaadati Apr 1, 2022
8fb693a
Merge branch 'title-with-friendly-api-name' of https://github.com/goo…
dansaadati Apr 5, 2022
481a153
Merge branch 'title-with-friendly-api-name' of https://github.com/goo…
dansaadati Apr 5, 2022
a11becb
Merge branch 'title-with-friendly-api-name' of https://github.com/goo…
dansaadati Apr 5, 2022
daa5d46
chore(deps): update dependency click and black together (#365)
dandhlee Mar 29, 2022
709c853
chore(deps): update dependency protobuf to ~=3.20.0 (#366)
renovate-bot Apr 1, 2022
d7470a6
feat(ruby): add friendly api name to page title
renovate-bot Apr 1, 2022
c77894a
Merge branch 'main' into title-with-friendly-api-name
dandhlee Apr 7, 2022
8181f80
chore(deps): update dependency google-cloud-core to ~=2.3.0 (#371)
renovate-bot Apr 11, 2022
da85678
fix: determine title in preprocessor instead of partial template (#370)
dansaadati Apr 12, 2022
044183e
chore(deps): update dependency google-cloud-storage to ~=2.3.0 (#372)
renovate-bot Apr 13, 2022
1746065
feat(ruby): add friendly api name to page title
dansaadati Apr 1, 2022
f4446c5
feat(ruby): add friendly api name to page title
dansaadati Apr 1, 2022
45a7ad7
feat(ruby): add friendly api name to page title
renovate-bot Apr 1, 2022
21defef
fix: update goldens to match newline remove on </h1>
dansaadati Apr 13, 2022
205c633
Merge branch 'main' into title-with-friendly-api-name
dansaadati Apr 13, 2022
0431dc1
fix: remove extra line in template
dansaadati Apr 13, 2022
0712881
factor out setting property type
dansaadati Apr 7, 2022
410544f
factor out property type name from ManagedReference pre-renderer
dansaadati Apr 7, 2022
0d3fcd5
remove dependency on partials/title
dansaadati Apr 7, 2022
343c2a9
modify to access first element in name
dansaadati Apr 7, 2022
225ca90
edit title in common.js based on type
dansaadati Apr 8, 2022
899de43
chore: write title in preprocessor instead of partial template
dansaadati Apr 8, 2022
3e745da
fix minor bugs, add new line on </h1> to match golden
dansaadati Apr 8, 2022
7404c03
fix: use model type instead of javaType, loosen title restriction in …
dansaadati Apr 11, 2022
eec6698
feat(Ruby): add intro sentence to summary with friendly api name
dansaadati Apr 13, 2022
e6f0e1a
feat(Ruby): add back in title update for staging purposes
dansaadati Apr 13, 2022
50fc32c
feat: add intro sentence with friend api name to summary
dansaadati Apr 14, 2022
8962579
fix: fix typo in documentation
dansaadati Apr 14, 2022
bee4860
Update third_party/docfx/templates/devsite/UniversalReference.common.js
dansaadati Apr 14, 2022
af2ddf2
Update third_party/docfx/templates/devsite/common.js
dansaadati Apr 14, 2022
a475c36
fix: add friendlyApiName to Ruby testdata
dansaadati Apr 15, 2022
d272521
fix: update goldens for Ruby with summary and title updates
dansaadati Apr 15, 2022
df33ebd
fix: declare <p> tag for intro sentence in template
dansaadati Apr 15, 2022
43f816a
Merge branch 'main' into update-summary-api-name
dansaadati Apr 27, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ exports.transform = function (model) {
}
}

if (model.summary != null && model.friendlyApiName) {
model.summary = common.addIntroSentenceToSummary(model.summary, model.friendlyApiName, model.type, model.name)
}
return model;
}

Expand Down Expand Up @@ -102,7 +105,7 @@ function handleItem(vm, gitContribute, gitUrlPattern) {
vm.sourceurl = common.getViewSourceHref(vm, null, gitUrlPattern);

// set to null incase mustache looks up
vm.summary = vm.summary || null;
vm.summary = vm.summary != null ? vm.summary : null;
Copy link
Contributor

Choose a reason for hiding this comment

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

How are the two different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows us to modify a summary that is an empty string. Without the distinction, we won't be able to add this sentence for pages contain an empty summary, but do have a summary key in the yaml.

vm.remarks = vm.remarks || null;
vm.conceptual = vm.conceptual || null;
vm.syntax = vm.syntax || null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ exports.getOptions = function (model) {
return {
"bookmarks": urefCommon.getBookmarks(model)
};
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{{!Copyright (c) Microsoft. All rights reserved. Licensed under the MIT license. See LICENSE file in the project root for full license information.}}
{{!master(layout/_master.tmpl)}}

<h1 class="page-title">{{title}}</h1>
<h1 class="page-title">{{#friendlyApiName}}{{friendlyApiName}} - {{/friendlyApiName}}{{title}}</h1>
{{#isNamespace}}
{{>partials/uref/namespace}}
{{/isNamespace}}
Expand Down
25 changes: 25 additions & 0 deletions third_party/docfx/templates/devsite/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ exports.isAbsolutePath = isAbsolutePath;
exports.isRelativePath = isRelativePath;

exports.getTitleForTypeProperty = getTitleForTypeProperty;
exports.addIntroSentenceToSummary = addIntroSentenceToSummary;

function getFileNameWithoutExtension(path) {
if (!path || path[path.length - 1] === '/' || path[path.length - 1] === '\\') return '';
Expand Down Expand Up @@ -107,6 +108,30 @@ function getTitleForTypeProperty(type, name, uid) {
}
}

/**
* Updates summary with an intro sentence containing the friendly API name.
*
* @param {string} summary The summary for the item.
* @param {string} type The type for the item (i.e. class, module).
* @param {string} friendlyApiName The friendly API title for the item.
* @param {Array} name The names for the item, only the first item is used for forming the sentence.
*/
function addIntroSentenceToSummary(summary, friendlyApiName, type, name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks fairly complicated compared to adding it to the mustache template. How about doing this in the template instead with something like...

{{#summary}}
{{#friendlyApiName}} Reference documentation and code samples for the {{friendlyApiName}} {{title}}.{{/friendlyApiName}}
{{summary}}
{{/summary}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gets complicated when including the object's type in the sentence. I factored out the type addition to title so we wouldn't have to have a partial file for that. This would be similar: we'd need to have a partial for switching based on type.

In general, I think we should try to avoid hard-coding language in the template -- it makes things like language updates and localization efforts more difficult.

Copy link
Contributor

Choose a reason for hiding this comment

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

We won't be doing localization based on the template given here. That's fine.

Another consideration is to add this for now into the Ruby YAMLs to begin with, which would be easier to find all the values rather than trying to compute and figure out what we need to extract from the YAMLs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline, in summary:
There are trade-offs between at generator level or template level. There may be more complexity to implement this at the language-generator level, but it'd also simplify our template. We can revisit this approach if we expand this update to other languages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand. Feels funny to be dealing with <p> tags here when we have the HTML template, which should handle... the HTML? If we want to come up with the content here to avoid the type complexity, we should at least leave off the <p> and </p> and add those in the template.

Copy link
Contributor Author

@dansaadati dansaadati Apr 15, 2022

Choose a reason for hiding this comment

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

🤦🏻‍♂️, right that'd be a good idea! I put the <p> tags in the template, let me know what you think.

if (summary == null || !type || !name) return summary;
if (!titlePrefixForTypeProperty[type]) return summary;

summaryIntro = "<p>"
summaryIntro += "Reference documentation and code samples for the ";
summaryIntro += friendlyApiName;
summaryIntro += " ";
summaryIntro += titlePrefixForTypeProperty[type].toLowerCase();
summaryIntro += " ";
summaryIntro += name[0].value;
summaryIntro += "."
summaryIntro += "</p>";
return summaryIntro + summary;
}

var gitUrlPatternItems = {
'github': {
// HTTPS form: https://github.com/{org}/{repo}.git
Expand Down