Kit Updates #77
Kit Updates #77
Conversation
This reverts commit 66b16a6.
This reverts commit ee7866a.
This reverts commit 0220314.
package.json
Outdated
| }, | ||
| "dependencies": { | ||
| "@github/octicons-react": "1.2.0-alpha.0a0d8862", | ||
| "classnames": "^2.2.5", | ||
| "d3-shape": "^1.2.0", | ||
| "react": "^16.2.0", | ||
| "react-router-dom": "^4.3.1", |
There was a problem hiding this comment.
I tried to understand react-router-dom to see if it's related to the demo and only needed to be in devDependencies?
I couldn't figure it out, so just posing the question here. ✌️
There was a problem hiding this comment.
👍 Unless we're using it in any of the classes exported by src/index.js, it should only be a dev dependency.
|
@broccolini to get pages working you have to add /docs to the end of the url, it;s because we had to rename Index.js to doc.js to get HMR working so the static build is generating a docs folder with an html file in it instead of a plain index.html. I've documented that & another url issue in #84 |
examples/docs.js
Outdated
| </nav> | ||
| <Route path='/docs/components' component={ComponentPage} /> | ||
| <Route path='/docs/demos' component={DemoPage} /> | ||
| <Route path='/docs/sandbox' component={Sandbox} /> |
There was a problem hiding this comment.
Routes are created based on files with x0 so you don’t need to add routes. You can do this but it will actually stop pages being statically rendered which is handled by x0
There was a problem hiding this comment.
^ @emplums This was showing as a pending comment so I'm unsure if you already saw it and whether the #77 (comment) is responding to it 🙈
There was a problem hiding this comment.
I was confused by this. Does it effectively alias those paths to wherever the ComponentPage, DemoPage, and Sandbox pages end up?
There was a problem hiding this comment.
@broccolini I tried not including s first and it wasn't working 🤷♂️ All the x0 docs say is To link between different components, install react-router-dom and use the Link component. (I'm using NavLink here to get the active styling, but I've tried with plain Links too). I'm not sure if the file structure needs to be set up for a certain way to get this to work correctly, our x0 config needs something extra to get this to work, or if there's a bug?
The only way I could get this working was to set up <Route> components as well. It works on the built gh pages site too. I can ask the x0 team for further details in getting <Link> to work out of the box, but any changes to routing ends up taking a while to test that it doesn't break HMR and also doesn't break the static build so I'd like to wait until after the release to fix that if that's cool!
There was a problem hiding this comment.
Just kidding I take everything back 😹 It looks like issue was that built in routing only works from _app.js, if we move all the navigation there it works fine! Plus HMR still works, plus we finally have some permalinks that work 🙀 I don't think we need a docs.js/index.js file at all, all of that can go right in _app.js instead.
There's still a few things that need to be addressed with this change post-release:
- setting the
basenamefor the library components to/primer-react/componentsmakes the URLs correct on the static site so that permalinks work, but it breaks HMR locally. - We still have an issue with the base url for gh-pages being
/primer-reactwhich doesn't load anything, we have to navigate to one of the pages/primer-react/componentsto see content.
I think both of those issues can be easily solved though! 🎉
This PR is getting quite large so I'm going to stop here! There are still a few extra things I'd like to do (listed below) but this feels like a good stopping point :) Here's what I've done in this PR:
<head>into_app.js(including links & meta data in package.json was not working)Issue: #80
To do in future PR:
Changes to our development workflow:
component-examplesfolder and include an example for your new component.index.jsfile incomponent-examplescc @primer/ds-core