content
content copied to clipboard
docs(manifest): Improve `icons` member page
Description
This work is part of improving the web/manifest docs. This PR focuses on the icons member page.
Some notable changes in this PR:
- Removed the "Type" table from the top of the page. The type can be covered in the Syntax section and does not need a table. (On some other pages like file_handlers, this Type table overcrowds with technology status banners.)
- Created a "Syntax" section with "Values" and "Properties" subsections. Replaced the table describing the values with a definition list format.
- Created a "Description" section to include details about security and performance aspects.
- Added an explanation for the example.
Motivation
To better explain the properties and example and make the page compliant with our page templates
Additional details
Spec links:
- https://w3c.github.io/manifest/#icons-member
- https://w3c.github.io/manifest/#declaring-multiple-icons
Related issues and pull requests
Tracking issue: https://github.com/mdn/mdn/issues/560
display page PR: https://github.com/mdn/content/pull/34386
Preview URLs
/en-US/docs/Web/Manifest/display/en-US/docs/Web/Manifest/icons/en-US/docs/Web/Manifest/theme_color
External URLs (1)
URL: /en-US/docs/Web/Manifest/icons
Title: icons
- https://w3c.github.io/manifest/ (2 times) (Note! This may be a new URL 👀)
(comment last updated: 2024-09-16 22:08:34)
Hi @hamishwillee, thanks a lot for the review and valuable inputs.
Sorry, I was able to get back to this PR only now so you might need to refresh your memory again on this. I've addressed all your comments and indicated parts that are slightly different from the suggestions. For sizes, I've proposed an updated version. Could you take another look when you have a moment? Thanks!
Just a nudge to bring this back on your reviewing radar when you get the chance :). Thanks!
Thanks, Hamish! This is great feedback and I believe the page is looking much better now. I've addressed your comments, plus made some additional changes:
- In the "Values" section, I've replaced the property terms with the placeholder values. I've adjusted the introductory prose to reflect this change.
- Added more info to the
<purpose-keywords>description based on the spec. And also clarified the behavior if there are invalid purposes. - Added two spec links to "See also"
@dipikabh Aside from https://github.com/mdn/content/pull/34725#discussion_r1751312240 I think this is pretty good. If you compare https://pr34725.content.dev.mdn.mozit.cloud/en-US/docs/Web/Manifest/icons with the original https://developer.mozilla.org/en-US/docs/Web/Manifest/icons you can see that the old way was "prettier" but this contains a lot more information and appears to be a more extensible pattern.
That said, we should update at least one of your other examples with just key-value to confirm the pattern works before merging this. Seem reasonable?
That said, we should update at least one of your other examples with just key-value to confirm the pattern works before merging this. Seem reasonable?
Hi @hamishwillee, I've applied the formatting to two other members:
- https://pr34725.content.dev.mdn.mozit.cloud/en-US/docs/Web/Manifest/theme_color
- https://pr34725.content.dev.mdn.mozit.cloud/en-US/docs/Web/Manifest/display
If it makes it simpler, I can gt rid of the Syntax section (and upgrade Keys to H2).
New structure looks good to me. I added a few comments/suggestions. The most important is https://github.com/mdn/content/pull/34725/files#r1759389846
Thanks, Hamish, for your time and patience in ironing out the Syntax block format and brainstorming all the Values/Keys/Properties shenanigans.
Appreciate you taking another close look at the rest of the content. The latest set of updates are in.