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

Use of tabpanes that obviates the need for Prettier/linter-ignore directives #3126

Merged

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Aug 2, 2023

  • Waiting on Tabpane: improve key-kind default setting google/docsy#1643 -- without which the changed tabbed panes don't get persisted.
  • Contributes to Find a better way to use tabpane shortcodes #3127
  • This proof-of-concept (POC) PR illustrates a proposal to simplify how we make use of tabpane shortcodes. The goal is to:
    • Eliminate the need for checker and linter directives like
      <!-- markdownlint-disable -->,
      <!-- prettier-ignore-start -->, etc.
    • Improve IDE and other tooling support: most tabbed panes in the docs contain code. Part of the problem with the current encoding is that the tools don't know that the content of a tab is code. The proposal illustrated here is to use code-block fences. I feel that the gains outweigh the extra code-fence lines that we need to write.
  • Updates docsy theme again.

WDYT @svrnm @cartermp et al.? If y'all agree, I'll make a pass over the docs to make this change uniformly.

If you compare the before and after, the tabbed panes changed in this PR look exactly the same:

@chalin chalin requested review from a team and mx-psi and removed request for a team August 2, 2023 20:23
@chalin chalin force-pushed the chalin-im-better-tabpanes-2023-08-02 branch from 6d5ebba to e86c926 Compare August 2, 2023 20:26
@chalin chalin changed the title Use of tabpanes that obviates Prettier and linter ignore directives Use of tabpanes that obviates the need for Prettier/linter-ignore directives Aug 2, 2023
Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

I like it.

@svrnm
Copy link
Member

svrnm commented Aug 3, 2023

I like this very much, not only does it improve the experience within IDEs and for our workflows, but I think it also separates 2 concerns that shouldn't be mixed (tabbing + code).

👍

@svrnm
Copy link
Member

svrnm commented Aug 3, 2023

One more thing, I think this might be solving: I am currently going over the Java manual instrumentation and I want to create a tabbing for Maven & Gradle setup, right now I don't know how to set titles for tabs that are different from the language used if each tab has their own language (here XML and kotlin).

So, this should be fixed here, right?

@chalin
Copy link
Contributor Author

chalin commented Aug 3, 2023

@svrnm - you can see an example of how to do that already in the JS docs, e.g.:

{{< tabpane lang=shell >}}

{{< tab TypeScript >}}
npm install typescript \
  ts-node \
  @types/node \
  express \
  @types/express
{{< /tab >}}

{{< tab JavaScript >}}
npm install express
{{< /tab >}}

{{< /tabpane >}}

In the case you mention, you'll want to set persist=header for the tabpane.

@chalin chalin force-pushed the chalin-im-better-tabpanes-2023-08-02 branch from e86c926 to 215c91b Compare August 3, 2023 13:03
@svrnm
Copy link
Member

svrnm commented Aug 3, 2023

@chalin the example uses the same language across both tabs (shell), but what I need is different titles per tab (Maven, Gradle) and different languages per tab (XML, Kotlin), which I don't see as a capability right now.

@chalin chalin force-pushed the chalin-im-better-tabpanes-2023-08-02 branch from 215c91b to 5b425aa Compare August 3, 2023 13:30
@chalin chalin requested a review from a team August 3, 2023 13:30
@@ -278,3 +278,11 @@ body.td-page--draft .td-content {
max-width: max-content;
}
}

// TODO(@chalin): upstream
Copy link
Member

Choose a reason for hiding this comment

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

what does this mean?

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 means that I'll be upstreaming that change to the Docsy repo after google/docsy#1641 gets merged.

@chalin chalin marked this pull request as draft August 3, 2023 13:39
@chalin chalin force-pushed the chalin-im-better-tabpanes-2023-08-02 branch from 5b425aa to e59193c Compare August 3, 2023 20:37
@chalin chalin marked this pull request as ready for review August 3, 2023 20:38
@chalin
Copy link
Contributor Author

chalin commented Aug 3, 2023

@svrnm @cartermp - this is ready for review. I'd suggest we merge, and I'll continue working on top of these changes in followup PRs.

@cartermp cartermp merged commit af0eca3 into open-telemetry:main Aug 3, 2023
@chalin chalin deleted the chalin-im-better-tabpanes-2023-08-02 branch August 3, 2023 23:13
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.

4 participants