-
Notifications
You must be signed in to change notification settings - Fork 328
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
[As part of release] Updates the docs to point to the right files #1466
Conversation
0eb8483
to
c6eb7ee
Compare
c6eb7ee
to
8691f96
Compare
8691f96
to
6b02958
Compare
6b02958
to
d71668a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks like a sane set of changes 👍
I've got one question about importing JavaScript components…
Unless we change the default branch to a v2.X tag merging this would make the installation docs incorrect until we do the v3.0.0 release so I've put a blocked label on this. |
d71668a
to
e8bf88a
Compare
I've addressed @36degrees comment on not having two options to import js. Hopefully this is now in a state to be ready to merge once we do a v3 release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Just one comment.
@@ -20,8 +20,6 @@ Components must: | |||
|
|||
Component folder and files should be singular, except in cases where they are more commonly used in groups, for example, radios, breadcrumbs and checkboxes. | |||
|
|||
An example component exists in `src/components/component-example`. | |||
|
|||
Use this as the basis for creating new components. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the "Use this as the basis..." line be removed as well? 'This' seems to refer to the example component that we've now removed reference to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I'll update this now.
Great spot 👀
As part of namespacing, we need to update the links the docs point to so that they are still valid and do not break. We also needed to update the examples to show how to use the new namespace.
e8bf88a
to
d409da8
Compare
As part of namespacing, we need to update the links the docs point to
so that they are still valid and do not break.