-
Notifications
You must be signed in to change notification settings - Fork 3
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
Demos #6
Demos #6
Conversation
Looks pretty good. One thing I would like to prevent is forcing users to copy the controls javascript and css files into the demo directory. Hmm, eventually we're going to offer packaging, so maybe we should just copy all of the files and directories from a controls folder. What do you think? The main issue I see is resolving the paths from the demo folder demo.html to the controls js and css. I think the simplest way would be to create another route that will catch those relative ../ path resource requests. |
That crossed my mind as well. If we use a demo.html in the element's folder (rather than a demo subfolder) then we could copy everything and avoid the issue or resolving relative paths. This keeps the user from having to make copies in the demo subdir, but it means the registry is going to copy more than it needs to run the demo. This is not a bad solution in my mind, because it's unlikely that the developer will store a lot of content in the element's folder that isn't directly related to the demo (at least given the current system). If we are concerned about pulling more data than necessary, we could require an explicit file-list in the demo value of the xtag.json. |
I'm not really concerned with pulling too much data. Right now I'm thinking that we should pull all the files because in the future we want to provide packaging, so that would requiring pulling in the controls css and js. We also wanted to add a test page that would function pretty the same as the demo page that you created. This would allow the user to create jasmine tests or use any other client side testing framework they want. So if we went the route of pulling all files, I think the only changes would be to crawl all files and directories in the repo, maybe rename XtagDemoAssest to something more generic like XTagFileAssest?? and then filter by the path when pulling files for /demo/ or /test/. We would still have to resolve the ../slider.js reference issue. Special route? or find and replace in the .HTML ? |
That all sounds good. Could we force the user to keep the demo.html at the same level as any If not, we could set the base url for the document to a url like Changing the URLs isn't impossible either; we could pass the demo HTML as a On Mon, Oct 1, 2012 at 5:49 PM, Arron Schaar notifications@github.comwrote:
|
I don't like the idea of putting everything at the same level b/c it then becomes unclear which files are for the demo, tests or the actual control. I think the folder requirement is a good way of keeping things organized. I like the idea of setting a base Url like /user/repo/element/version?/assets/ that would catch all file requests and figure out what to do. What about the x-tag.js? There is a chance of that being in different places for each element. For example, in mozilla/x-tag-elements, we have it located at lib/x-tag/x-tag.js. However, I doubt that's the same for most other people creating one off elements in their own repositories. It sounds like we have to have a custom route to match all of their assets and change the src for the x-tag.js reference. What do you think? |
While reading expressjs docs, I found a way to clean up that asset route: app.param('range', /^(\w+)\.\.(\w+)?$/);
app.get('/range/:range', function(req, res){
var range = req.params.range;
res.send('from ' + range[1] + ' to ' + range[2]);
}); It might be possible to make a custom regexp param for the last bit (the path with slashes). |
@pfraze Oh like the app.param() stuff. That's way better than the regex. |
I added github basic auth so we don't run into API limits as quickly. Can you rebase? |
Sure. I'll add the param update while I'm at it. On Tue, Oct 16, 2012 at 1:59 PM, Arron Schaar notifications@github.comwrote:
|
FYI, I've been working with your pull request and added some of the functionality we talked about before. Namely making the element demos assets more generic (renamed to XtagElementAsset) and pulling in all resources for a given element. I'll let you know when I have something worthy of sharing. |
Ok, so should I hold off on making any changes? On Wed, Oct 17, 2012 at 3:18 PM, Arron Schaar notifications@github.comwrote:
|
I'm going to check some stuff in today on a different branch. It's not going to be finished but it has the main features stubbed out. The main changes are:
|
Ok cool. Why are you rewriting the src and href paths? I thought my last commit On Wed, Oct 17, 2012 at 5:53 PM, Arron Schaar notifications@github.comwrote:
|
Shoot!! did it? I didn't dive in to see how that worked. I removed it for now, but that does sound like a better way of handling it. The new base path for assets will be /assets/:tagid/:path, so I'm assuming we can use the same mechanism you implemented for that right? I checked in my stuff @ https://github.com/pennyfx/x-tag-registry/tree/assets Have a look. I'm going to be away for the next few days, but feel free to change out the base path stuff for demo and get that working again. Sorry about that. If you want to tackle anything else that I mentioned that would be cool too. |
No problem. I'll take a look and get some stuff committed before friday. On Wed, Oct 17, 2012 at 6:23 PM, Arron Schaar notifications@github.comwrote:
|
I'm back from vacation and was going to checkout how you did the basepath stuff, but /js/demo.js is missing. Can you check that in? Thanks |
Hey, I'm sorry but I'm swamped with my other work. I'm not going to be able On Wed, Oct 24, 2012 at 12:52 PM, Arron Schaar notifications@github.comwrote:
|
No problem. I based my latest changes on your branch. This is what I have so far. http://registry.x-tags.org/mozilla/x-tag-elements/x-accordion/view Thanks for your help. |
You can test this build at http://linkshui.com:3000/