-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[Docs] Add versions to docs site #3383
Conversation
@mbrookes! 🎉 This looks awesome, I'm going to look more indepth tonight.
I think this is a clever idea and worth exploring. 👍 I hope you're enjoying your trip! 😄 |
Thanks. Work, and nowhere exciting, but got to catch up with some old friends for a couple of hours last night which was great!
Glad you like the idea. Wasn't sure how big an overhead it would add though. |
@@ -0,0 +1,83 @@ | |||
import React from 'react'; | |||
import ReactDOM from 'react-dom'; |
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.
I wonder how far back this will work since pre React 0.14.0 it was React.render
instead of ReactDOM.render
?
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.
Unless I'm misunderstanding, this is going to import whatever version of React is current when the versions.jsx
js is built.
It does make the rather large assumption that it's possible to have two scripts (versions.js
and app.js
) that are running different versions of React loaded concurrently, and I hadn't researched whether that was realistic. It seems it isn't by default, but there is a hack to enable this: facebook/react#1939 (comment)
The alternative might be to not import React at all in versions.js
, and to depend on the version of React loaded by any particular version of the Material-UI demo-site. This assumes that the DropDown and the parts of Material-UI it uses are compatible with the earliest version of React in use.
If neither of those is possible, then the only other option (that uses Material-UI) would be to go back and edit the source for each prior version to add the dropdown using the version of Drop Down Menu, and therefore React, that was current at the time.
I'd rather avoid that if possible (editing the source of all the prior versions), so a native <select>
(styled to look as close to Material-UI) and with plain js is the only other option I can think of.
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.
And regardless - while navigating around old versions would be a bonus, in reality if you're using a particular version, you'll navigate to it and stick with it, so I don't think getting this working is a priority for 0.15.0. Having the old version docs available should be enough.
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.
Humm, that sounds far too complicated. What about killing this versions.jsx
? Do we need this DropDownMenu
for older versions? I don't think so.
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.
Yeah, like I said, it's a nice-to-have. I'll kill it for now. We have more important things to worry about!
Nice! @mbrookes I didn't get to check this PR out and review it as in depth as I wanted to tonight! I got caught up working on another issue 😫. Gonna get some rest now but I'll give this a spin during the day tomorrow. Initial thought when looking over it: |
Edited:
I'd thought that we could perhaps add them as non-default options to the menu. But I don't think we should add them for prior releases, and I think we should remove them in the major that the RCs are for.
The script already covers most of what's required, but we would need to watch for changes on github.com, pull updates, and have write access to push to the I wasn't going to tackle that as part of this PR. |
@@ -81,6 +114,23 @@ const AppLeftNav = React.createClass({ | |||
<div style={prepareStyles(styles.logo)} onTouchTap={this.handleTouchTapHeader}> | |||
Material-UI | |||
</div> | |||
|
|||
<DropDownMenu |
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.
Makes sense, maybe separate we could consider setting up a separate travis build for |
Oh, interesting - I hadn't ever looked at Travis-CI beyond clicking to review failed checks, and kicking myself for forgetting to lint before pushing (again!). 😁 |
@callemall/material-ui Okay - I've added pre-release support. Pre-releases don't get symlinked in the The gh-pages-build script also now supports building a HEAD version. It takes care of making sure HEAD is at the top of There's also now a -p option, which will force push If someone could please take this for a spin? Something like:
|
@mbrookes I have just tried this PR: http://oliviertassinari.github.io/material-ui/v0.15.0/.
|
Thanks @oliviertassinari - do you still have the console output, and if so, please could you put it in a gist? It will help me see where things are going wrong. |
I have started again from a clean git history and a clean folder structure.
|
@oliviertassinari - I'm sorry, I just don't get it. I've been trying to break this for the last four hours without success! From the message it's as if Could you perhaps add Perhaps you could also |
I'm going to try this now... I'm tempted to make @oliviertassinari paint @mbrookes a picture if it works, and @mbrookes remove an unnecessary component if it doesn't. 🎲 🎌 |
Alright I'm running into a separate issue that's probably my fault. Maybe I should paint everyone a picture and get back to work... |
LOL 😆 |
Okay the results are in!
Edit: I'm going to head home now from office, I'll play around with this some more later and provide more constructive feedback. I'm excited to get this finished and merged! Awesome stuff @mbrookes |
I did wonder, but when I searched, it seemed like
I'd say. :) You need gh-pages from my fork for the past versions and syminked current version. |
@oliviertassinari Might that be permission related? |
@@ -31,11 +31,11 @@ The roadmap is a living document, and it is likely that priorities will change, | |||
- [ ] [[1321](https://github.com/callemall/material-ui/pull/1321#issuecomment-174108805)] Composable AppBar component. | |||
- [ ] [[#2493](https://github.com/callemall/material-ui/pull/2493)] New Stepper component. | |||
- [ ] [[#3132](https://github.com/callemall/material-ui/pull/3132)] Scrollable Tabs. | |||
- [X] [[#3033](https://github.com/callemall/material-ui/pull/3132)] New Subheader component. | |||
- [x] [[#3033](https://github.com/callemall/material-ui/pull/3132)] New Subheader component. |
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.
I've been meaning to open a PR for this but since you're editing this line anyway, could you update the link to the right PR? 3132 -> 3133? Technically some of the links above it are wrong too so we could also just fix this later/separately...
Could we adjust the approach a bit? I'm thinking that instead of using the version command line argument to build the docs in Maybe running the script without a version argument can checkout the latest from master then commit it to Do you guys think that makes sense? |
Makes sense to me! 👌
How does Travis distinguish between submitted PRs / pushes (for testing), and merged PRs (for building), or do we then consider building the docs site part of the testing?
It does already, but how does that get from Travis' fork back to the callemall gh repo? That's why I added the -p argument - I assume it'll need to push (?), but only for merges, The Travis docs are super-unhelpful on this. |
Oh geez! I have fallen victim to nonsense thinking in the late hour 😪 I assumed we already were automating releases using Travis, on second look it seems like we're not. So yes that would be an issue. Travis does support an npm
Not completely sure. I figured Travis clones the repo from callemall/material-ui, not from a separate fork. So it's upstream should be here and it should be able to push directly. There are a lot uncertainties with what I proposed. I'd say start with the change checking out from git tag first and maybe I'll be smarter after some sleep 😄 |
Mmm, yeah, I meant clone, not fork. And I don't know why I thought the remote wouldn't be set correctly - I have a local I've updated to use tags for versions. I haven't tested as thoroughly as the previous version, but for four runs through (tagged version, HEAD, HEAD, later tagged version) it behaves as expected. Trying to build the same tagged version twice will fail, since the directory already exists in a commit, but that's no bad thing. |
var execSync = require('child_process').execSync; | ||
|
||
var usage = '\nbuild <vn.n.n[-pre[.n]] | HEAD> [-p]\n'; | ||
var versionsFile = './src/www/versions.json'; |
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.
@mbrookes Always try resolving these relative paths using path.resolve(__dirname, './src/...')
because relative paths are resolved relative to current working directory, so if someone does this: node docs/gh-pages-build.js
from the root directory this won't work.
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.
Well, much of the git wrangling assumes we're in docs
, and it would get super messy and hard-to-read if we path.resolve
all of those.
I was assuming this is going to be called in an automated fashion either for HEAD builds, or as part of @hai-cea's release process, or, if run manually, by using the npm start
script.
No harm in making it fool proof, but much easier to process.chdir('__dirname');
though I think?
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.
Well, much of the git wrangling assumes we're in docs, and it would get super messy and hard-to-read if we path.resolve all of those.
I see, well you're right, I didn't take those into account 😅
No harm in making it fool proof, but much easier to process.chdir('__dirname'); though I think?
That's actually a very smart move 👍 Yeah I think it's a lot safer 👍
If there's nothing further, lets squash, merge and get alpha.1 released. We'll get an idea of and refinements needed after the first time it gets used for real. |
I'm good 👍 👍 |
I'm good with that too! |
[Docs] Add versions to docs site
Awesome 😍 I'll update, rebase and tidy up the release notes tomorrow 👍 |
@callemall/material-ui
Guys, I might be busy this week, but I haven't been completely idle! 😄
WIP but throwing this out for comment. You might find this easier to test if you merge with
master
temporarily, as the script will bounce you back there; otherwise comment out that line.You will also need the
gh-pages
branch from my repo which has all the past versions in it. (I can share the script for building that, but it has no future value).The script
docs/gh-pages-build
does the work of adding a new versioned build of the docs site to thegh-pages
branch, and updatingversions.json
which is used for the versions Drop Down Menu (currently in the LeftNav).When future versions are added, older versions (starting from
0.15.x
) will be able to navigate to them, sinceversions.json
is shared across all versions that use it.To build a new version, you can run the script with
npm run gh-pages-build
, or justnode ./gh-pages-build.js
. It takes one argument - a version version number for the build beginning withv
.Feedback appreciated!☺️
There is also an additional commit, which is a WIP POC for adding version navigation to the older versions.
It occurred to me that we could retrofit a standalone component to the old versions of the docs site by having it in a separate
js
file with its own render target in the HTML, then being absolutely positioned, somewhere along the topNavBar
.I have updated
index.html
(only in master, not in the older versions yet), and partially extracted the necessary code for a standaloneVersions
component. Where I got stuck was with the webpack config; something I know nothing about. I was hoping that if this approach seems viable, one of you guys could take a look and see what's required.Edit: Should add that I haven't styled the DropDown in the LeftNav to fit - not convinced it looks right there anyway, but it was convenient for testing.
Closes #1986.