fix: determine title in preprocessor instead of partial template #370

dansaadati · 2022-04-08T16:26:17Z

This change moves setting the title to the preprocessor rather than within the template. This should improve readability around the page-title element of the template, as well as make it easier to reuse the title in other parts of generated HTML.

dandhlee · 2022-04-08T23:12:40Z

third_party/docfx/templates/devsite/ManagedReference.common.js

+      if (model.javaType) {
+        model.title = common.getTitleForTypeProperty(model.javaType, model.name, model.uid)


Should we use model.type instead since if there is model.javaType it overwrites it to model.type?

Yes, thanks! model.type has been overwritten by model.javaType at this point, so we don't need an extra branch here.

dandhlee · 2022-04-08T23:17:57Z

third_party/docfx/templates/devsite/ManagedReference.html.primary.tmpl

@@ -1,7 +1,8 @@
 {{!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}}{{title}}{{/title}}{{^title}}{{>partials/title}}{{/title}}</h1>
+<h1 class="page-title">{{#title}}{{title}}{{/title}}


I think it's now safe to just write

<h1 class="page-title">{{title}}</h1>

instead of

<h1 class="page-title">{{#title}}{{title}}{{/title}}</h1>

dandhlee · 2022-04-08T23:23:33Z

third_party/docfx/templates/devsite/ManagedReference.common.js

+    var typePropertyName = getTypePropertyName(model.type)
+    model[typePropertyName] = true;


What happens when we encounter a type that's not specified below?

This would set model[typePropretyName] for types not covered in the switch statement, or set model[undefined] = true; in the case it's not a valid type. I think we should keep the behavior the same as before, so I'll revert this to have model[typePropertyName] be set on a case by case basis.

tbpg

+1 to the questions from @dandhlee. Otherwise LGTM.

…template

dandhlee · 2022-04-11T17:48:50Z

third_party/docfx/templates/devsite/ManagedReference.html.primary.tmpl

@@ -1,7 +1,8 @@
 {{!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}}{{title}}{{/title}}{{^title}}{{>partials/title}}{{/title}}</h1>
+<h1 class="page-title">{{title}}
+</h1>


Is the </h1> closing in a separate line needed?

In order to match our current generated HTML, yes. We can put it on a separate line as well, this would then be updated for all our documentation as well.

The "expected" at the moment is on a separate line. Let's fix this.

tbpg · 2022-04-11T19:17:56Z

third_party/docfx/templates/devsite/common.js

+    "exception": "Exception"
+}
+
+function getTitleForTypeProperty(type, name, uid) {


Will you add a comment saying why we have this function?

dandhlee

LGTM.

(My heart almost dropped when I saw 600 file changes. oof 😂)

dandhlee

No changes since last approval.

dansaadati · 2022-04-12T19:31:53Z

LGTM.

(My heart almost dropped when I saw 600 file changes. oof 😂)

😭, may the force be with us 🙏.

* factor out setting property type * factor out property type name from ManagedReference pre-renderer * remove dependency on partials/title * modify to access first element in name * edit title in common.js based on type * chore: write title in preprocessor instead of partial template * fix minor bugs, add new line on </h1> to match golden * fix: use model type instead of javaType, loosen title restriction in template * fix: remove newline in <h1>, document getTitleForTypeProperty function Co-authored-by: Dan Lee <[email protected]>

…#367) * feat(ruby): add friendly api name to page title * feat(ruby): add friendly api name to page title * chore(deps): update dependency click and black together (#365) * chore(deps): update dependency protobuf to ~=3.20.0 (#366) * feat(ruby): add friendly api name to page title * chore(deps): update dependency google-cloud-core to ~=2.3.0 (#371) * fix: determine title in preprocessor instead of partial template (#370) * factor out setting property type * factor out property type name from ManagedReference pre-renderer * remove dependency on partials/title * modify to access first element in name * edit title in common.js based on type * chore: write title in preprocessor instead of partial template * fix minor bugs, add new line on </h1> to match golden * fix: use model type instead of javaType, loosen title restriction in template * fix: remove newline in <h1>, document getTitleForTypeProperty function Co-authored-by: Dan Lee <[email protected]> * chore(deps): update dependency google-cloud-storage to ~=2.3.0 (#372) [![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [google-cloud-storage](https://togithub.com/googleapis/python-storage) | `~=2.2.0` -> `~=2.3.0` | [![age](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/compatibility-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/confidence-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>googleapis/python-storage</summary> ### [`v2.3.0`](https://togithub.com/googleapis/python-storage/blob/HEAD/CHANGELOG.md#230-httpsgithubcomgoogleapispython-storagecomparev221v230-2022-04-12) [Compare Source](https://togithub.com/googleapis/python-storage/compare/v2.2.1...v2.3.0) ##### Features - add dual region bucket support and sample ([#748](https://togithub.com/googleapis/python-storage/issues/748)) ([752e8ab](https://togithub.com/googleapis/python-storage/commit/752e8ab42d23afd68738e4d7ca6cdeee416dfd50)) - track invocation id for retry metrics ([#741](https://togithub.com/googleapis/python-storage/issues/741)) ([bd56931](https://togithub.com/googleapis/python-storage/commit/bd5693164e7331df5f14186fd002e72e5203d7ee)) ##### Bug Fixes - **deps:** drop pkg_resources ([#744](https://togithub.com/googleapis/python-storage/issues/744)) ([e963f33](https://togithub.com/googleapis/python-storage/commit/e963f33ced2852b64d721d69928b54443461ec9c)) ##### Documentation - fix links in blob module ([#759](https://togithub.com/googleapis/python-storage/issues/759)) ([9b29314](https://togithub.com/googleapis/python-storage/commit/9b2931430b0796ffb23ec4efacd82dacad36f40f)) ##### [2.2.1](https://togithub.com/googleapis/python-storage/compare/v2.2.0...v2.2.1) (2022-03-15) ##### Bug Fixes - remove py.typed marker file for PEP 561 ([#735](https://togithub.com/googleapis/python-storage/issues/735)) ([f77d2f7](https://togithub.com/googleapis/python-storage/commit/f77d2f787f435f2f898e9babcdab81225672ad4f)), closes [#734](https://togithub.com/googleapis/python-storage/issues/734) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/doc-templates). * feat(ruby): add friendly api name to page title * feat(ruby): add friendly api name to page title * feat(ruby): add friendly api name to page title * fix: update goldens to match newline remove on </h1> * fix: remove extra line in template Co-authored-by: Dan Lee <[email protected]> Co-authored-by: WhiteSource Renovate <[email protected]>

* feat(ruby): add friendly api name to page title * feat(ruby): add friendly api name to page title * chore(deps): update dependency click and black together (#365) * chore(deps): update dependency protobuf to ~=3.20.0 (#366) * feat(ruby): add friendly api name to page title * chore(deps): update dependency google-cloud-core to ~=2.3.0 (#371) * fix: determine title in preprocessor instead of partial template (#370) * factor out setting property type * factor out property type name from ManagedReference pre-renderer * remove dependency on partials/title * modify to access first element in name * edit title in common.js based on type * chore: write title in preprocessor instead of partial template * fix minor bugs, add new line on </h1> to match golden * fix: use model type instead of javaType, loosen title restriction in template * fix: remove newline in <h1>, document getTitleForTypeProperty function Co-authored-by: Dan Lee <[email protected]> * chore(deps): update dependency google-cloud-storage to ~=2.3.0 (#372) [![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [google-cloud-storage](https://togithub.com/googleapis/python-storage) | `~=2.2.0` -> `~=2.3.0` | [![age](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/compatibility-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/google-cloud-storage/2.3.0/confidence-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>googleapis/python-storage</summary> ### [`v2.3.0`](https://togithub.com/googleapis/python-storage/blob/HEAD/CHANGELOG.md#230-httpsgithubcomgoogleapispython-storagecomparev221v230-2022-04-12) [Compare Source](https://togithub.com/googleapis/python-storage/compare/v2.2.1...v2.3.0) ##### Features - add dual region bucket support and sample ([#748](https://togithub.com/googleapis/python-storage/issues/748)) ([752e8ab](https://togithub.com/googleapis/python-storage/commit/752e8ab42d23afd68738e4d7ca6cdeee416dfd50)) - track invocation id for retry metrics ([#741](https://togithub.com/googleapis/python-storage/issues/741)) ([bd56931](https://togithub.com/googleapis/python-storage/commit/bd5693164e7331df5f14186fd002e72e5203d7ee)) ##### Bug Fixes - **deps:** drop pkg_resources ([#744](https://togithub.com/googleapis/python-storage/issues/744)) ([e963f33](https://togithub.com/googleapis/python-storage/commit/e963f33ced2852b64d721d69928b54443461ec9c)) ##### Documentation - fix links in blob module ([#759](https://togithub.com/googleapis/python-storage/issues/759)) ([9b29314](https://togithub.com/googleapis/python-storage/commit/9b2931430b0796ffb23ec4efacd82dacad36f40f)) ##### [2.2.1](https://togithub.com/googleapis/python-storage/compare/v2.2.0...v2.2.1) (2022-03-15) ##### Bug Fixes - remove py.typed marker file for PEP 561 ([#735](https://togithub.com/googleapis/python-storage/issues/735)) ([f77d2f7](https://togithub.com/googleapis/python-storage/commit/f77d2f787f435f2f898e9babcdab81225672ad4f)), closes [#734](https://togithub.com/googleapis/python-storage/issues/734) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/doc-templates). * feat(ruby): add friendly api name to page title * feat(ruby): add friendly api name to page title * feat(ruby): add friendly api name to page title * fix: update goldens to match newline remove on </h1> * fix: remove extra line in template * factor out setting property type * factor out property type name from ManagedReference pre-renderer * remove dependency on partials/title * modify to access first element in name * edit title in common.js based on type * chore: write title in preprocessor instead of partial template * fix minor bugs, add new line on </h1> to match golden * fix: use model type instead of javaType, loosen title restriction in template * feat(Ruby): add intro sentence to summary with friendly api name This adds a friendly intro sentence to the page's summary. This sentence is only generated if the yaml includes a "friendlyApiName" key. * feat(Ruby): add back in title update for staging purposes * feat: add intro sentence with friend api name to summary * fix: fix typo in documentation * Update third_party/docfx/templates/devsite/UniversalReference.common.js Co-authored-by: Dan Lee <[email protected]> * Update third_party/docfx/templates/devsite/common.js Co-authored-by: Dan Lee <[email protected]> * fix: add friendlyApiName to Ruby testdata * fix: update goldens for Ruby with summary and title updates * fix: declare <p> tag for intro sentence in template Co-authored-by: Dan Lee <[email protected]> Co-authored-by: WhiteSource Renovate <[email protected]>

dansaadati added 7 commits April 7, 2022 15:33

factor out setting property type

0156b05

factor out property type name from ManagedReference pre-renderer

a606968

remove dependency on partials/title

76fef0f

modify to access first element in name

a384bd0

edit title in common.js based on type

10abb0e

chore: write title in preprocessor instead of partial template

2cef6d2

fix minor bugs, add new line on </h1> to match golden

6d5cc8a

dansaadati requested review from a team and tbpg April 8, 2022 16:26

dandhlee changed the title ~~chore: write title in preprocessor instead of partial template~~ fix: determine title in preprocessor instead of partial template Apr 8, 2022

dandhlee reviewed Apr 8, 2022

View reviewed changes

tbpg reviewed Apr 11, 2022

View reviewed changes

fix: use model type instead of javaType, loosen title restriction in …

2f472bc

…template

dansaadati requested a review from dandhlee April 11, 2022 17:45

dandhlee reviewed Apr 11, 2022

View reviewed changes

dansaadati requested a review from dandhlee April 11, 2022 18:40

tbpg approved these changes Apr 11, 2022

View reviewed changes

fix: remove newline in <h1>, document getTitleForTypeProperty function

0d98f9d

dandhlee approved these changes Apr 12, 2022

View reviewed changes

Merge branch 'main' into factor-title-to-preprocessor

a4094dc

dansaadati requested a review from dandhlee April 12, 2022 19:25

dandhlee approved these changes Apr 12, 2022

View reviewed changes

dansaadati merged commit 9610e67 into main Apr 12, 2022

dansaadati deleted the factor-title-to-preprocessor branch April 12, 2022 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: determine title in preprocessor instead of partial template #370

fix: determine title in preprocessor instead of partial template #370

dansaadati commented Apr 8, 2022

dandhlee Apr 8, 2022

dansaadati Apr 11, 2022

dandhlee Apr 8, 2022

dansaadati Apr 11, 2022

dandhlee Apr 8, 2022

dansaadati Apr 11, 2022

tbpg left a comment

dandhlee Apr 11, 2022

dansaadati Apr 11, 2022

dandhlee Apr 11, 2022

dansaadati Apr 12, 2022

tbpg Apr 11, 2022

dansaadati Apr 12, 2022

dandhlee left a comment

dandhlee left a comment

dansaadati commented Apr 12, 2022

		if (model.javaType) {
		model.title = common.getTitleForTypeProperty(model.javaType, model.name, model.uid)

		var typePropertyName = getTypePropertyName(model.type)
		model[typePropertyName] = true;

fix: determine title in preprocessor instead of partial template #370

fix: determine title in preprocessor instead of partial template #370

Conversation

dansaadati commented Apr 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dandhlee left a comment

Choose a reason for hiding this comment

dandhlee left a comment

Choose a reason for hiding this comment

dansaadati commented Apr 12, 2022