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 Web Share API to GroupData.json [ADVICE FIRST BEFORE MERGE!] #8454

Merged
merged 1 commit into from
Aug 31, 2021

Conversation

hamishwillee
Copy link
Collaborator

For #8381 I created a "Web Share API" doc. This API includes navigator.canShare() and navigator.share() methods.

This PR updates the groupdata to add a new sidebar menu for "Web Share API" with those two methods. I had expected to add this new API to both the methods.

Before I do ... (or merge this) ... the two methods currently link to "{{APIRef("HTML DOM")}}". Is it the "right thing" to do to create this sidebar and apply it to those methods, or can I somehow have both sidebars?

@hamishwillee hamishwillee requested review from a team and fiji-flo and removed request for a team August 30, 2021 06:46
@wbamberg
Copy link
Collaborator

wbamberg commented Aug 31, 2021

For #8381 I created a "Web Share API" doc. This API includes navigator.canShare() and navigator.share() methods.

This PR updates the groupdata to add a new sidebar menu for "Web Share API" with those two methods. I had expected to add this new API to both the methods.

Before I do ... (or merge this) ... the two methods currently link to "{{APIRef("HTML DOM")}}". Is it the "right thing" to do to create this sidebar and apply it to those methods, or can I somehow have both sidebars?

I don't know how much of this you know already, so apologies if you do.

There are 2 main sidebar macros in use in Web/API (excluding all the custom ones like https://github.com/mdn/yari/blob/main/kumascript/macros/WebRTCSidebar.ejs ). These are DefaultAPISidebar and APIRef.

  • DefaultAPISidebar is used for "API overview pages", like the "Web Sharing API" page you're creating in https://github.com/mdn/content/pull/8381/files#diff-21debc64f150e2ef15ed0075d9de3d0936b9d33152e1e9249a72fe2243b98205 (and any API guide pages you create under that one). This sidebar is driven primarily by GroupData.json.

  • APIRef is used for interface, method, property, event etc pages, and is driven primarily by URL structure (i.e. the pages that are children or siblings of this page). It does, however, as you say, accept an argument which references a GroupData entry. If that's given, the things in the group appear under the APIRef sidebar under "Related pages for XYZ" (see for example
    the bit at the bottom of the sidebar on Related pages for Fetch API, which lists Request and stuff, although the main sidebar is about Window).

So yeah: when you land this change, navigator.canShare() and navigator.share() should do {{APIRef("Web Share API")}}. But they'll still have almost the same sidebar that they have now, they'll just have a little bit at the end that lists the other share method:

Screen Shot 2021-08-30 at 5 27 08 PM

You may also be interested in https://discourse.mozilla.org/t/defaultapisidebar-apiref-and-groupdata/40210 in which I tried to drag a lot of this into the light.

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

I've looked at this, and it looks fine. I've also tested it, and tried it with a copy of the overview page from #8381, and it gives me the expected sidebar on that page:

Screen Shot 2021-08-30 at 5 34 31 PM

@hamishwillee
Copy link
Collaborator Author

Thanks very much @wbamberg . I was not aware of this to that level of detail. I'd only tested on the API level page.

@hamishwillee hamishwillee merged commit c14912c into mdn:main Aug 31, 2021
@hamishwillee hamishwillee deleted the groupdata_webshare branch August 31, 2021 01:16
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