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

Retire CanvasSidebar from en-US #22586

Merged
merged 6 commits into from
Dec 6, 2022
Merged

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Nov 28, 2022

This PR replaces the custom sidebar CanvasSidebar in en-US. When it's merged I will deprecate this sidebar.

  • The PR takes advantage of the "Tutorial" section that authors can now add to DefaultAPISidebar, as of feat(macros/DefaultAPISidebar): add Tutorial section yari#7646.

  • For that to work we need to add translations strings for the "Tutorial" label, which is done in this PR by adding an entry to L10n-Common.json. For the translations I've copied strings out of CanvasSidebar.ejs, and in this PR asked for feedback from l10n maintainers (see below in this comment). I've had feedback from es and fr. For the other locales I think we can wait until they remove CanvasSidebar, so their feedback should not have to block this PR.

  • We also need to list the tutorial pages in groupdata, and this PR does that, too.

  • Then we need to replace CanvasSidebar macro calls with DefaultAPISidebar.

  • Finally finally, Prettier noise.

Please note that in its original incarnation this PR was only about updating l10n strings and asking for feedback, before I decided that we might as well make the changes here too. I've kept the original PR description below.


Original PR description:

I'm working on retiring two custom sidebars, CanvasSidebar and WebGLSidebar, and replacing them with the standard DefaultAPISidebar.

These custom sidebars contain a multi-page tutorial, which DefaultAPISidebar didn't know how to present. In mdn/yari#7646 I updated DefaultAPISidebar to support a "Tutorial" section. But this has an l10n dependency, because it needs a localized string to label the tutorial:

Screen Shot 2022-11-28 at 10 29 46 AM

That's what this PR adds. It is incomplete and probably wrong, because I don't speak these languages, but it is a start and I hope the l10n experts can fix my errors :).

I've also updated GroupData.json with the tutorial data for Canvas, and updated the Canvas overview page to use DefaultAPISidebar, so you can see what it looks like. The page for http://localhost:3000/fr/docs/Web/API/Canvas_API, for instance, looks like:

Screen Shot 2022-11-28 at 10 35 07 AM

In this PR I have just copied strings from the translations in https://github.com/mdn/yari/blob/main/kumascript/macros/CanvasSidebar.ejs. Not all locales have a translation there, and I think adding strings for those missing locales is not a blocker for this work, since it's not a regression. But I will happily accept them in this PR of course.

So my request of l10n experts is:

  • if this PR does contain a string in your language, check it's correct and add a correction if needed.
  • if this PR does not contain a string in your language, add one (although like I say I won't block this PR on adding missing strings)

@mdn/localization-team-leads

@github-actions github-actions bot added the Content:WebAPI Web API docs label Nov 28, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 28, 2022

Co-authored-by: Alexander <alexander_belial999@hotmail.com>
Copy link
Member

@SphinxKnight SphinxKnight left a comment

Choose a reason for hiding this comment

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

✔️ for fr

@wbamberg wbamberg changed the title Add "Tutorial" l10n string for DefaultAPISidebar Retire CanvasSidebar from en-US Dec 5, 2022
@wbamberg wbamberg marked this pull request as ready for review December 5, 2022 19:56
@wbamberg wbamberg requested review from a team as code owners December 5, 2022 19:56
@wbamberg wbamberg requested review from sideshowbarker and bsmth and removed request for a team December 5, 2022 19:56
Copy link
Contributor

@teoli2003 teoli2003 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 these changes! Thank you, Will.

@teoli2003 teoli2003 merged commit 6e5f4ae into mdn:main Dec 6, 2022
hamishwillee pushed a commit to hamishwillee/content that referenced this pull request Dec 12, 2022
* Add 'Tutorial' to l10n strings

* Update groupdata with Canvas tutorial; Update Canvas API main page to use DefaultAPISidebar

* Trim a couple of guides

* Add es string

Co-authored-by: Alexander <alexander_belial999@hotmail.com>

* Update all Canvas pages to use standard sidebar

Co-authored-by: Alexander <alexander_belial999@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants