mdn / sprints

Archived: MDN Web Docs issues are tracked in the content repository.
https://github.com/mdn/content
Creative Commons Zero v1.0 Universal
149 stars 142 forks source link

Update web manifest docs so we have a separate page for each manifest key #1676

Closed chrisdavidmills closed 5 years ago

chrisdavidmills commented 5 years ago

It would be good to organize https://developer.mozilla.org/en-US/docs/Web/Manifest better.

We should model these docs and compat data after the way @wbamberg has done manifest.json docs for WebExtensions, see https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json and https://github.com/mdn/browser-compat-data/tree/master/webextensions/manifest.

bershanskiy commented 5 years ago

I'll work on this. For some reason, the list of sub-pages does not update. Am I doing something wrong?

wbamberg commented 5 years ago

For some reason, the list of sub-pages does not update. Am I doing something wrong?

You need to shift-refresh the page while logged in to make it re-evaluate macros (and then normal-refresh the page again after giving it time to finish rebulding the page).

I just did and it looks fine (except that you have code styling on the macro call which looks a bit odd).

Thanks for doing this work @bershanskiy !

bershanskiy commented 5 years ago

I reorganized (moved member descriptions to dedicated pages) and extended the documentation (added all parts from standard format similar to that of WebExtensions manifest members). As noted on the main page, serviceworker, screenshots, and iarc_rating_id still don't have documentation pages (I'll work on that when I have time). In the mean time, I would like someone to look over the already completed docs and give me any feedback.

bershanskiy commented 5 years ago

Update: reorganization complete.

I reorganized (moved member descriptions to dedicated pages) and extended the documentation (added all parts from standard format similar to that of WebExtensions manifest members).

The pages probably would benefit form someone else's review and feedback.

@chrisdavidmills Is this issue ready to be closed?

chrisdavidmills commented 5 years ago

@bershanskiy great work, thanks so much! I've edited all the member subpages, and think they are ready to go.

Before we close this, I am wondering what would be best to do about a sidebar for this page. @wbamberg , do you have any thoughts on this? I was thinking about using GroupData/DefaultAPISidebar to do this, but not sure it'd work. I guess the other option would be to just use a simple set of quicklinks for now.

bershanskiy commented 5 years ago

@chrisdavidmills Thanks for reply.

I guess the other option would be to just use a simple set of quicklinks for now.

Obviously, I don't have as much experience as you do, but I don't think this is a good solution. After all, specification can change and add/remove some attributes (it's still a draft). It would be a pain to manually update all sidebars and this would be incredibly error-prone. This issue would be even worse for any translations of the docs.

wbamberg commented 5 years ago

@wbamberg , do you have any thoughts on this?

What do you want the sidebar to contain? Just the list of manifest keys or are there any other overview/guide pages? It seems like under https://developer.mozilla.org/en-US/docs/Web/Manifest there are just keys, no guides, so maybe a simple alphabetical listing of links to the subpages would be good enough.

I was thinking about using GroupData/DefaultAPISidebar to do this, but not sure it'd work.

I don't think this is a good idea. It's nice to avoid yet another sidebar macro, but GroupData/DefaultAPISidebar definitely assumes its users are Web APIs, meaning (1) they live under Web/API and (2) they have certain subtle (undocumented) constraints on things like page names and tags and subpages. In this case it seems like Web/Manifest is a new top-level bit of information architecture, so would not sit happily there. For similar reasons I don't like the presence of "Web Components" in GroupData.

It would be great if we could generalise the GroupData/DefaultAPISidebar/APIRef thing so it was reliably usable by more docsets in MDN, but that's not on the cards at the moment.

I guess the other option would be to just use a simple set of quicklinks for now.

You mean, like hardcoded in each page? Yeah, again I appreciate the desire to avoid more macros, but I agree with @bershanskiy that this would be too hard to maintain.

What about:

{{QuickLinksWithSubpages("/en-US/docs/Web/Manifest")}}

This would get you something like: https://developer.mozilla.org/en-US/docs/User:wbamberg/test-page.


Edited to add: or is that what you meant by "a simple set of quicklinks"?

chrisdavidmills commented 5 years ago

Beautiful, thanks @wbamberg. I didn't actually know about that macro ;-)

I have implemented it on https://developer.mozilla.org/en-US/docs/Web/Manifest and subpages. Looks pretty good to me.

bershanskiy commented 5 years ago

This looks good to me. Is there anything else that needs to be done or can we close this issue?

ExE-Boss commented 5 years ago

I updated the page to use a hand crafted sidebar using {{ListSubpagesForSidebar(…)}}

bershanskiy commented 5 years ago

@ExE-Boss What is the difference between the two macros? As far as I see, they produce the same result, but they are very different on the outside. Also, did you mean to update only the main page or each member's page as well? I'd like to keep pages consistent (use the same macro), so if ListSubpagesForSidebar is better, I'll update all the details pages.

Also, while editing you created a section title "Subnav" that would show up in the nav bar, I removed it (I hope I didn't break anything else in the mean time).

ExE-Boss commented 5 years ago

Actually, that broke pages that were transcluding the navbar using {{Page("/en-US/docs/Web/Manifest", "Subnav")}}.

Also, the difference is that {{QuicklinksWithSubpages(…)}} produces a list of subpages without the parent page wrapped in a <section id="Quick_Links"> element, whereas {{ListSubpagesForSidebar(…)}} creates a list where you have to supply your own <section id="Quick_Links"> wrapper element.

bershanskiy commented 5 years ago

@ExE-Boss I undid my last commit and restored the article to your version.

chrisdavidmills commented 5 years ago

@ExE-Boss so how is this better? It strikes me that the difference is only minor and cosmetic, whereas the implementation is now more confusing because the features used to implement the sidebar on the main versus the child pages is now inconsistent and the macro calls are less intuitive.

I wish you'd mention potential changes first and start a discussion, rather than just deciding to go and do things with no discussion.

ExE-Boss commented 5 years ago

I think we should modify {{QuicklinksWithSubpages(…)}} to use {{ListSubpagesForSidebar(…)}} and put the top level page at the top in bold.

wbamberg commented 5 years ago

how is this better?

It shows Web App Manifest at the top of the sidebar and has the subpages in code style. It does look nicer. On the other hand:

On balance I don't think it is worth it.

I think we should modify {{QuicklinksWithSubpages(…)}} to use {{ListSubpagesForSidebar(…)}} and put the top level page at the top in bold.

Perhaps. But please let's make that a separate discussion, not here.

ExE-Boss commented 5 years ago

Technically, the Subnav header isn’t completely required (you can do {{Page("…", "Quick_Links")}}), but then the Page macro call also needs to be wrapped in a <section id="Quick_Links"> element.

chrisdavidmills commented 5 years ago

In any case, this got fixed, so closing. Thanks for all the great work @bershanskiy !

wbamberg commented 5 years ago

Based on https://github.com/mdn/sprints/issues/1676#issuecomment-500482337, I have reverted the 2 pages whose sidebar was changed to use handcoded quicklinks. Now all pages just use QuicklinksWithSubpages.

@ExE-Boss , if you do want to propose further changes here, please talk about them outside the context of this issue, and let's talk about them before making more changes.