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 drafted content for 'Building your own JavaScript components' #468

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

romaricpascal
Copy link
Member

@romaricpascal romaricpascal commented Oct 8, 2024

Important

This PR should only be merged after the next release of GOV.UK Frontend v5.

Adds the documentation for the public JavaScript API parts implemented so far, which help you build and initialise your own components.

Fixes #467

Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for govuk-frontend-docs-preview ready!

Name Link
🔨 Latest commit 2a892c5
🔍 Latest deploy log https://app.netlify.com/sites/govuk-frontend-docs-preview/deploys/67079eb676262600091cb093
😎 Deploy Preview https://deploy-preview-468--govuk-frontend-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@romaricpascal romaricpascal force-pushed the build-your-own-components branch 2 times, most recently from b49b9da to 37281db Compare October 8, 2024 09:51
- helps [`createAll` initialise them](#using-createall-with-your-components)
- supports consistent error messaging

If your project supports native static properties, the `moduleName` property can be defined as a native static property:
Copy link
Member Author

Choose a reason for hiding this comment

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

@calvin-lau-sig7 This looks like a remnant from before we had the section explaining how to implement static properties. I think we can drop it and just have the example following the list.

This comment was marked as off-topic.

Copy link
Member Author

@romaricpascal romaricpascal Oct 8, 2024

Choose a reason for hiding this comment

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

Oh sorry, I mean dropping the whole sentence altogether rather than trying to reduce it. It use to be:

If your project supports native static properties

<CODE EXAMPLE>

Otherwise

<OTHER CODE EXAMPLE>

Because it applied to many code examples, we added that first 'Defining static properties' on components and wrote all the examples with the native syntax, making sure there's a link to the 'Defining static properties' section when we introduce a static property (for the moduleName here, and for the elementType in 'Customising the expected type of the root element').

I was thinking that because of that the sentence can be completely removed.

Suggested change
If your project supports native static properties, the `moduleName` property can be defined as a native static property:

Copy link
Contributor

@calvin-lau-sig7 calvin-lau-sig7 Oct 8, 2024

Choose a reason for hiding this comment

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

Ahh yes, sorry I misunderstood.

Sure, that's fine. Though I'd suggest moving the bullet list after the example, to make a better intro for the example and it's clear what it's showing. I'll make a suggestion.


#### Using `this.$root` with TypeScript

Because `govuk-frontend` does not provide type definitions in its package, TypeScript may give the following error message: “Property ‘$root’ does not exist on type &lt;NAME_OF_YOUR_CLASS&gt;”.
Copy link
Member Author

Choose a reason for hiding this comment

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

Is it worth putting the message in a blockquote, or maybe at least on its own line to avoid the '<NAME_OF_YOUR_CLASS>' part going awkwardly to the line?

Here's how it looks like with a blockquote:

Screenshot with the error message as a blockquote

Here's how it looks like as its own paragraph (quotes courtesy of the <q> element):

Screenshot with the error message as its own paragraph

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just put it as its own paragraph here – this quote is an interesting one, because it's showing a message to look for, rather than something we want the service team to tell the user (the usual reason we put things in quotes).

Copy link
Member Author

@romaricpascal romaricpascal Oct 8, 2024

Choose a reason for hiding this comment

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

this quote is an interesting one

As in 'we should keep it'? Hadn't seen the next suggestion, sorry.

@calvin-lau-sig7
Copy link
Contributor

calvin-lau-sig7 commented Oct 8, 2024

Okay, I've had a look through and it looks pretty good as far as I can tell!

Tech docs involve a lot of backtick styling, code snippets, etc. but everything I see looks pretty readable, and consistent with the rest of the Frontend docs.

The 2i review suggested that some of the intro paragraphs could be a bit more specific about what's happening in the examples, but @seaemsi agreed it's probably one to look at after publishing.

I'm happy to approve once we've got everything added in and reviewed.

EDIT: I'll also give a proofread this afternoon, should be just minor things like sentence flow, style, contractions, etc.

@romaricpascal romaricpascal force-pushed the build-your-own-components branch from 3ef308f to 5375535 Compare October 8, 2024 13:05
@romaricpascal romaricpascal added documentation Improvements or additions to documentation javascript Pull requests that update Javascript code labels Oct 8, 2024
@romaricpascal romaricpascal force-pushed the build-your-own-components branch from 0ac885b to 76dc05d Compare October 8, 2024 17:28
@romaricpascal
Copy link
Member Author

@calvin-lau-sig7 Thanks for the edits, accepted all of them wholesale I think appart from one for which I left the conversation unresolved above.

@romaricpascal romaricpascal force-pushed the build-your-own-components branch from 76dc05d to 2a892c5 Compare October 10, 2024 09:30
@romaricpascal romaricpascal merged commit 9ee5e89 into main Oct 10, 2024
5 checks passed
@romaricpascal romaricpascal deleted the build-your-own-components branch October 10, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation javascript Pull requests that update Javascript code 🚀 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new 'Building your own components' page
3 participants