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 "wireframe" for generated event docs #454

Closed
wants to merge 16 commits into from
Closed

Conversation

DebbieAtSeam
Copy link
Contributor

No description provided.

@DebbieAtSeam DebbieAtSeam requested a review from a team as a code owner December 11, 2024 19:13
@DebbieAtSeam DebbieAtSeam marked this pull request as draft December 11, 2024 19:13
@DebbieAtSeam DebbieAtSeam marked this pull request as ready for review December 11, 2024 21:11
@DebbieAtSeam DebbieAtSeam requested a review from a team as a code owner December 11, 2024 21:11
Key of the [climate preset](../../capability-guides/thermostats/creating-and-managing-climate-presets) that was activated.
</details>
<details>
<summary>is_fallback_climate_preset <code>boolean</code></summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

GitBook does not seems to be rendering the links inside the blocks
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other things are not being rendered inside these expand/collapse blocks either. See https://docs.seam.co/latest/api/acs/systems#location for an example of "`" not rendering as code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The details block is HTML, right? If so, do we need to use HTML tags inside to get it to render?

Copy link
Contributor

Choose a reason for hiding this comment

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

That renders the URL now, but the url is going to https://github.com/seamapi/docs/blob/main/docs/capability-guides/thermostats/setting-and-monitoring-temperature-thresholds/README.md

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Other things are not being rendered inside these expand/collapse blocks either. See docs.seam.co/latest/api/acs/systems#location for an example of "`" not rendering as code.

Sounds like a GitBook issue and we will need to add a markdown render step for content that is inside html tags.

@razor-x
Copy link
Contributor

razor-x commented Dec 11, 2024

Overall looks good!

  1. Just realized that GitBook does generate anchor links for each expandable section, so that's cool! (https://docs.seam.co/latest/~/revisions/TltbxcE7HlDD4UFMenCt/api/thermostats#temperature_celsius-number)
  2. Not sure what to do about the markdown not rendering inside the blocks. Are the links malformed or do we need to try <a> tags?
  3. How to deal with common event properties like event_id? Do we leave those out here and only document them on the Events page, or repeat them for each event, or?

@DebbieAtSeam
Copy link
Contributor Author

DebbieAtSeam commented Dec 11, 2024

Overall looks good!

  1. Just realized that GitBook does generate anchor links for each expandable section, so that's cool! (https://docs.seam.co/latest/~/revisions/TltbxcE7HlDD4UFMenCt/api/thermostats#temperature_celsius-number)

Yay!

  1. Not sure what to do about the markdown not rendering inside the blocks. Are the links malformed or do we need to try <a> tags?

Was wondering the same thing. Can we try HTML instead of MD inside these blocks?

  1. How to deal with common event properties like event_id? Do we leave those out here and only document them on the Events page, or repeat them for each event, or?

I vote for repeating them for completeness if it's something that can be done programmatically/automatically.


<details>
<summary>method <code>enum</code></summary>
Method used to adjust the thermostat manually. `seam` indicates that the Seam API, Seam CLI, or Seam Console was used to adjust the thermostat.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@razor-x OK, this seems inconsistent with the other page that has expand/collapse blocks, but using "``" or "" works here to format text as code.

</details>
<details>
<summary>upper_limit_celsius <code>number</code></summary>
Upper temperature limit, in °C, defined by the set <a href="https://docs.seam.co/latest/capability-guides/thermostats/setting-and-monitoring-temperature-thresholds">threshold</a>.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@razor-x As you saw, the relative URL does not work. Using the full/absolute URL here does not work either. What should we do?

@DebbieAtSeam
Copy link
Contributor Author

@razor-x Given the GitBook limitations that we've found, what's the next step for this PR?

@razor-x
Copy link
Contributor

razor-x commented Dec 12, 2024

If links are unable to render inside the expandable section, we only have two choices (unless we contact GitBook support and they make it work)

  1. Replace the expandable sections with something else in this PR.
  2. Strip out the urls inside these sections during the doc gen step. (Leave this PR as is)

@DebbieAtSeam
Copy link
Contributor Author

If links are unable to render inside the expandable section, we only have two choices (unless we contact GitBook support and they make it work)

  1. Replace the expandable sections with something else in this PR.
  2. Strip out the urls inside these sections during the doc gen step. (Leave this PR as is)

@razor-x Of these choices, I vote for 2. If also like to see what happens if I add an expand/collapse with a link through the GitBook UI. I wonder if it works there.

docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
docs/api/thermostats/README.md Outdated Show resolved Hide resolved
@DebbieAtSeam
Copy link
Contributor Author

DebbieAtSeam commented Dec 17, 2024

@razor-x Of course, it works when added through the GitBook GUI... and it looks identical to what we added through GitHub (that did not work).
a5578c4#diff-0ad85fee1e305f0af27a1d00939ecb6f2d99cd122bfff916d873fc62a4cf6106R127

Working: https://docs.seam.co/latest/api/noise_sensors#events

@DebbieAtSeam
Copy link
Contributor Author

DebbieAtSeam commented Dec 18, 2024

Conclusions:

  • To get code-formatted text within summary blocks, you need to use <code></code>.
  • To get code-formatted text within details blocks, but outside of summary blocks, you can use backticks.
  • Leaving spaces between lines is very important for getting links within details blocks to render correctly.
  • For relative links, make sure to include the README.md part of the URL if the absolute URL does not end in a specific filename.

@DebbieAtSeam DebbieAtSeam requested a review from razor-x December 18, 2024 22:48
@DebbieAtSeam
Copy link
Contributor Author

@razor-x What's the next step for this PR? Should we merge it, or should the generation template additions go in this PR too?

@razor-x razor-x marked this pull request as draft December 20, 2024 18:47
@razor-x
Copy link
Contributor

razor-x commented Dec 20, 2024

We can't merge this one because it has to be owned by the generation. But this PR will be used as a reference to update the template to match this format.

@DebbieAtSeam
Copy link
Contributor Author

@razor-x Should we close this PR now?

@razor-x razor-x closed this Jan 7, 2025
@github-actions github-actions bot deleted the event-wireframe branch January 22, 2025 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants