Tab widget (prototype) #1606

Simran-B · 2020-11-25T18:44:49Z

npm run dev to start server, then open http://localhost:4000/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions in browser.

himanshu211raj · 2020-11-25T19:01:32Z

@Simran-B i didn't understand what you mean to say? As i am a beginner I don't know much about open source.

Simran-B · 2020-11-25T22:54:19Z

@himanshu211raj Do you know JavaScript? If yes, then you can help me here. If no, then you can still help to add more examples (for example PowerShell scripts where currently only Bash examples are given). Or do you want to help with something else?

himanshu211raj · 2020-11-26T07:45:10Z

@himanshu211raj Do you know JavaScript? If yes, then you can help me here. If no, then you can still help to add more examples (for example PowerShell scripts where currently only Bash examples are given). Or do you want to help with something else?

Where i can help in adding more examples and in which other domains i will make contributions and how?

Simran-B · 2020-11-26T11:27:07Z

@himanshu211raj The "Setting an environment variable" example only works in a Bash shell for instance:

https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-environment-variable

You could add additional examples for PowerShell Core and Windows cmd. Here is an example how it should look:

https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#example-running-a-script-using-bash

If you are not familiar with PowerShell and cmd, then try to find something else that can be improved. You can find general information on how to contribute here:

https://github.com/github/docs/blob/main/CONTRIBUTING.md#contributing-to-this-repository-

github-actions · 2020-12-03T17:00:47Z

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

janiceilene · 2020-12-03T19:59:32Z

Thanks so much for opening a PR @Simran-B! I'll get this triaged for review 💖

Simran-B · 2020-12-04T13:06:44Z

@janiceilene Thanks, but this PR is still in draft state, i.e. too early for a review. I wanted to share my work in progress code so that others have a chance to help. I would appreciate early feedback nonetheless.

There are a couple of issues at the moment:

Tab groups aren't independent and tabs are currently switched based on name regardless of the group they belong to
The tab content isn't initialized (hidden) as desired
Clicking tabs lets the content jump (undesired scrolling), except for the first tabs instance (does preventDefault() help perhaps?)
Markdown support in tab titles is a bit hacky
Extended markdown might not work as tab content, but haven't tried

janiceilene · 2020-12-04T20:09:30Z

Thanks for the clarification @Simran-B! I moved it over to where the engineering team can take a look with that additional context 💖

github-actions · 2020-12-12T17:07:25Z

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

github-actions · 2020-12-21T17:19:37Z

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

Simran-B · 2020-12-21T23:44:15Z

Notes to self:

Implemented compensation for content shift when switching tabs that keeps the on-screen distance between the clicked tab and the top of the viewport constant.
Add group information to content, not just tab header, for easier selection?
Use classes or data attributes to indicate tab metadata? (browser support, performance)
Allow deep linking with anchors
- Tab link should be copyable
- Anchor link should activate the tab on load
- Clicking tabs should potentially set hash in address bar (add to window.history), despite preventDefault()
A11y? (tab index, ARIA roles, cursor support?)
Should nested tabs or too many tabs (how many?) be disallowed explicitly?
Should content between tab group tag and first tab tag raise an error?
~~Adjust CSS class for tabs based on number of tabs?~~ <nav class="UnderlineNav my-3"> (my-<amount>)
--> my-<n> only seems to determine the margin/padding above and below the tabs
~~Do extended markdown features only work inside of blocks which derive from the extended markdown tag?~~
--> That does not appear to be the case, admonitions and icons work inside of tabs just fine. Probably because the rendering of content is handled elsewhere, outside of the tab implementation.
Should probably render initial tab state into HTML (select first tab of each group?)
Remember preferred tab per group, using local storage?
Any frontmatter customization options that would be useful? Like defaultPlatform, but how to do that (esp. in frontmatter schema) if groups and tabs are defined per page?
Tests to potentially add:
- Tabs are rendered, only one tab active
- Correct activation of initial tab for each group and the corresponding content
- No jumping caused by switching tabs where above tab content would normally push the content down

github-actions · 2020-12-30T17:35:41Z

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

Simran-B · 2020-12-30T19:12:48Z

Still experimenting with this locally, making it cleaner and more robust.

Note to self: check what causes the unwanted Markdown processing of the HTML template and find a way to avoid it.

crichID · 2021-01-06T20:50:52Z

Still experimenting with this locally, making it cleaner and more robust.

@Simran-B thank you for continuing to experiment with this! I'm following along and looking forward to using this new widget in the docs.

vanessayuenn

@Simran-B A general-usage tab component has been on our radar for a while, so thank you for making it happen! This is looking great so far!

In terms of implementation, I know this is still early stage, but I am curious if you have considered a nested block approach, instead of the current liquid tag within a liquid block approach? I think this will result in modular and more maintainable code. From a usability perspective, a nested liquid block usage might be more user-friendly as well:

{% tabs "shell" %}

  {% tab "Bash" %}
    bash stuff
  {% endtab %}

  {% tab "PowerShell" %}
    powershell stuff
  {% endtab %}

{% endtabs %}

I am happy to elaborate some more if it's helpful. Please let me know how I can support you to get this to the finish line!

vanessayuenn · 2021-01-08T15:05:40Z

lib/liquid-tags/tabs.js

+    this.tabs.forEach(async tab => {
+      // Could use textOnly mode to strip out markup, but slugger does that and more, see pushBlock()
+      //await renderContent(tab.title, null, { textOnly: true })
+      tab.title = (await renderContent(tab.title, null)).slice(3, -4) // HACK: remove surrounding <p>


👍🏼 on slugger usage here! It's a good idea to support markdown in the tab header, but I think if we do, we should also support parsing variables/reusables here as well to be consistent. context will need to be passed in for the variables/reusables to be rendered properly. We could rearrange the order of calling these methods a bit:

Suggested change

tab.title = (await renderContent(tab.title, null)).slice(3, -4) // HACK: remove surrounding <p>

tab.title = slugger.slug(await renderContent(tab.title, context))

github-actions · 2021-01-15T18:08:17Z

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

Simran-B · 2021-01-19T16:58:25Z

Hi @vanessayuenn, thanks for the feedback. I missed your comment among all the notifications (and there are frequent spam comments in this repo unfortunately). Sorry for getting back so late.

I am curious if you have considered a nested block approach, instead of the current liquid tag within a liquid block approach?

I did, but the markup looks somewhat cleaner with tags and it (mostly) solves a particular problem: What to do with content between tab blocks? For example:

{% tabs "shell" %}

-- What to do with this text? --

  {% tab "Bash" %}
    bash stuff
  {% endtab %}

-- And this? --

  {% tab "PowerShell" %}
    powershell stuff
  {% endtab %}

-- And this? --

{% endtabs %}

With tabs as tags, only one corner case remains:

{% tabs "shell" %}

-- What to do with this text? --

  {% tab "Bash" %}
    bash stuff

  {% tab "PowerShell" %}
    powershell stuff

{% endtabs %}

From a usability point of view and especially with larger amounts of tabbed content, I can see how tab blocks would be better however. I'll see if I can intercept the callbacks for the content to throw an error if there's something outside of tab blocks.

# Conflicts: # lib/render-content/index.js

…offset in the viewport

…text to interpreted as Markdown code block

github-actions · 2021-01-27T17:27:29Z

This PR is stale because it has been open 7 days with no activity and will be automatically closed in 3 days. To keep this PR open, update the PR by adding a comment or pushing a commit.

Simran-B · 2021-01-27T19:13:18Z

@vanessayuenn Regarding nested blocks, are there any existing examples?

Registering both tabs and tab doesn't seem right, as the latter is only valid within the former. All state should be stored in the Tabs class, but I'm not sure how one would do that with two independent block types (wild guess: use the parent environment?). Maybe the current implementation could be kept, but adjusted to also expect endtab in unknownTag? Or is there a way to register the tab block within the tabs block only?

If there's no built-in functionality for nested block handling, then I'm not sure how to progress. The parser code is pretty daunting.

chiedo · 2021-04-05T13:05:57Z

@Simran-B sorry we've taken so long to get back to you on this. 🙇🏿 We're not going to move forward in this direction but we appreciate your contribution! Thanks for the time and effort you invested in this PR.

Simran-B · 2021-04-08T22:19:08Z

@chiedo No problem. I'd still like to know if it's a general decision against tabs and whether you have an alternative in mind. #2121 depends on a solution IMO, because adding a headline for every example (one per shell type) is messy.

I'll probably work on this further as an exercise when I find the time, as I need such a widget myself (for Jekyll, but I'm more comfortable with JavaScript than Ruby, so this is great for experimentation especially because the Liquid renderer is basically a port).

chiedo · 2021-04-09T14:31:55Z

That sounds good @Simran-B! @myarb can speak towards the product vision on tabs vs not having tabs. But from an engineering perspective I do know that we'll be working on changes that would influence how this solution was implemented.

myarb · 2021-04-09T17:03:26Z

👋🏼 @Simran-B we aren't currently planning to expand our tabs in docs, but this is helpful feedback for future docs changes. If you're still interested in working on the problem stated in #253, adding Windows-specific examples in areas where they're missing in a PR would be a welcome addition as Lucas mentioned in this comment 💜 Thanks so much for your contributions and passion for GitHub Docs!

* Update action text for PR * rephrase * rephrase

Simran-B mentioned this pull request Nov 25, 2020

Shell not indicated in some examples, missing alternative shell solutions #253

Closed

github-actions bot added the stale There is no recent activity on this issue or pull request label Dec 3, 2020

janiceilene removed the stale There is no recent activity on this issue or pull request label Dec 3, 2020

janiceilene added the engineering Will involve Docs Engineering label Dec 3, 2020

github-actions bot added the stale There is no recent activity on this issue or pull request label Dec 12, 2020

janiceilene removed the stale There is no recent activity on this issue or pull request label Dec 14, 2020

github-actions bot added the stale There is no recent activity on this issue or pull request label Dec 21, 2020

janiceilene removed the stale There is no recent activity on this issue or pull request label Dec 21, 2020

github-actions bot added the stale There is no recent activity on this issue or pull request label Dec 30, 2020

github-actions bot removed the stale There is no recent activity on this issue or pull request label Dec 31, 2020

chiedo assigned vanessayuenn Jan 7, 2021

vanessayuenn reviewed Jan 8, 2021

View reviewed changes

github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 15, 2021

janiceilene removed the stale There is no recent activity on this issue or pull request label Jan 15, 2021

Simran-B added 10 commits January 20, 2021 00:23

Tabs PoC

40956ad

# Conflicts: # lib/render-content/index.js

Sample content

09be17a

WIP

df1f798

Next step towards initial tab per group, comments

47a77c6

Prevent content from shifting away by maintaining the tab's relative …

6e460f2

…offset in the viewport

Add octicon test

4ee3b30

Adjust module import for render-content

c20e13b

render-content is now an object, renderContent is the desired function

384e66b

temp

143fba2

Returned string is rendered somewhere else, causing 4-space indented …

9017184

…text to interpreted as Markdown code block

Simran-B force-pushed the tab-widget branch from ea6fcd0 to 9017184 Compare January 19, 2021 23:25

github-actions bot added the stale There is no recent activity on this issue or pull request label Jan 27, 2021

github-actions bot removed the stale There is no recent activity on this issue or pull request label Jan 28, 2021

janiceilene added the waiting for review Issue/PR is waiting for a writer's review label Feb 5, 2021

github-actions bot closed this Feb 13, 2021

vanessayuenn reopened this Feb 15, 2021

heiskr closed this Mar 25, 2021

heiskr reopened this Mar 25, 2021

docubot assigned heiskr Mar 25, 2021

Merge branch 'main' into tab-widget

7d9d55b

vanessayuenn removed their assignment Apr 1, 2021

chiedo closed this Apr 5, 2021

Simran-B mentioned this pull request Apr 8, 2021

Actions: Add examples for shells other than Bash #2121

Closed

4 tasks

jnidzwetzki pushed a commit to jnidzwetzki/docs that referenced this pull request Oct 6, 2022

Update action text for PR (github#1606)

5ba6b75

* Update action text for PR * rephrase * rephrase

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tab widget (prototype) #1606

Tab widget (prototype) #1606

Simran-B commented Nov 25, 2020

himanshu211raj commented Nov 25, 2020

Simran-B commented Nov 25, 2020

himanshu211raj commented Nov 26, 2020

Simran-B commented Nov 26, 2020

github-actions bot commented Dec 3, 2020

janiceilene commented Dec 3, 2020

Simran-B commented Dec 4, 2020

janiceilene commented Dec 4, 2020

github-actions bot commented Dec 12, 2020

github-actions bot commented Dec 21, 2020

Simran-B commented Dec 21, 2020 •

edited

Loading

github-actions bot commented Dec 30, 2020

Simran-B commented Dec 30, 2020

crichID commented Jan 6, 2021

vanessayuenn left a comment

vanessayuenn Jan 8, 2021

github-actions bot commented Jan 15, 2021

Simran-B commented Jan 19, 2021

github-actions bot commented Jan 27, 2021

Simran-B commented Jan 27, 2021

chiedo commented Apr 5, 2021

Simran-B commented Apr 8, 2021 •

edited

Loading

chiedo commented Apr 9, 2021

myarb commented Apr 9, 2021

	tab.title = (await renderContent(tab.title, null)).slice(3, -4) // HACK: remove surrounding <p>
	tab.title = slugger.slug(await renderContent(tab.title, context))

Tab widget (prototype) #1606

Tab widget (prototype) #1606

Conversation

Simran-B commented Nov 25, 2020

himanshu211raj commented Nov 25, 2020

Simran-B commented Nov 25, 2020

himanshu211raj commented Nov 26, 2020

Simran-B commented Nov 26, 2020

github-actions bot commented Dec 3, 2020

janiceilene commented Dec 3, 2020

Simran-B commented Dec 4, 2020

janiceilene commented Dec 4, 2020

github-actions bot commented Dec 12, 2020

github-actions bot commented Dec 21, 2020

Simran-B commented Dec 21, 2020 • edited Loading

github-actions bot commented Dec 30, 2020

Simran-B commented Dec 30, 2020

crichID commented Jan 6, 2021

vanessayuenn left a comment

Choose a reason for hiding this comment

vanessayuenn Jan 8, 2021

Choose a reason for hiding this comment

github-actions bot commented Jan 15, 2021

Simran-B commented Jan 19, 2021

github-actions bot commented Jan 27, 2021

Simran-B commented Jan 27, 2021

chiedo commented Apr 5, 2021

Simran-B commented Apr 8, 2021 • edited Loading

chiedo commented Apr 9, 2021

myarb commented Apr 9, 2021

Simran-B commented Dec 21, 2020 •

edited

Loading

Simran-B commented Apr 8, 2021 •

edited

Loading