mizzao / meteor-sharejs

Meteor smart package for transparently adding ShareJS editors to an app
MIT License
225 stars 53 forks source link

Cannot change themes, language, etc. for editor. Routing issue? #7

Closed swese44 closed 10 years ago

swese44 commented 10 years ago

New to Meteor, hopefully I'm missing something obvious...

Followed the basic setup in the Readme, works as expected with default options. Then I tried to set the language to JavaScript and theme to twilight:

Template.editor.config = function() {
  return function(editor) {
    editor.setShowPrintMargin(false);
    editor.setTheme('ace/theme/twilight');
    editor.getSession().setMode("ace/mode/javascript");
    return editor.getSession().setUseWrapMode(true);
  };
};

Console shows JavaScript errors, Ace cannot parse the requested assets. The current URL is: /r/roomName

Ace is requesting assets at: /r/mode-javascript.js /r/theme-twilight.js

and these URLs return the application HTML. I'd assume it should be requesting these assets from the root URL, but I'm not sure.

I'm on Meteor 0.7.1. Here are the packages I'm using:

{
  "packages": {
    "bootstrap-3": {},
    "spin": {},
    "iron-router": {},
    "accounts-ui-bootstrap-3": {},
    "bootstrap-errors": {},
    "streams": {},
    "sharejs": {}
  }
}

Hopefully this is just a routing config issue. Any ideas? Thanks!

mizzao commented 10 years ago

Those ace assets aren't imported by this package (yet.) We should fix that, but I don't want to pollute the package with a lot of files. Any suggestions?

As you know, Meteor loads the default app for any route unless they are specifically handled by the middleware, defined as assets, etc.

swese44 commented 10 years ago

Thanks for the quick reply!

There's already a lot of files in this package and I don't think adding all the asset files would be a problem, as long as you could get them to load from the /public directory and only load them in via Ajax. Not sure if that is possible though, I'm just learning meteor.

I can just copy the static files into my /public directory for now. Any idea how to make Ace request the assets from the site's root directory instead of the current routed directory?

swese44 commented 10 years ago

Actually I'm not seeing an obvious way to do this. It looks like any customization for mode or language invokes scripts like /packages/sharejs/ace.js, which are not available since they aren't in the public directory (and actually the files it is requesting are deeply nested in subpackages). I wish I could be of more help but I'm just getting started meteor :)

mizzao commented 10 years ago

Can you point to an example of some code which tries to invoke ace.js at a specific path? I don't think it would be too hard to do this, but I'd like to do it right.

swese44 commented 10 years ago

So by trying to change the mode (language) or theme: editor.setTheme('ace/theme/twilight'); editor.getSession().setMode("ace/mode/javascript");

it hits the server at: /CURRENT_URL_DIRECTORY/mode-javascript.js /CURRENT_URL_DIRECTORY/theme-twilight.js

Meteor intercepts all requests if it doesn't match an asset in the /public directory, so I'm not sure how to get around that. I know you can declare which files are made available on the server and client in a smart package, but I'm not sure if you can declare files to be placed in the /public directory... probably not. You might have to manually place Ace assets in the /public directory, and make sure it makes its Ajax calls for those assets in the correct directory.

mizzao commented 10 years ago

Hi @swese44, that's good information. Does CURRENT_URL_DIRECTORY depend on the current route that is being hit, or is it a constant? If it changes depending on the page or route being rendered, that's bad.

The second thing you mentioned is not too hard to deal with. It's actually possible to declare static files anywhere in the directory structure. See https://github.com/mizzao/meteor-build-fetcher for an example.

swese44 commented 10 years ago

It depends on the current route. For example, my route was "/r/roomName" and the above code triggered requests to "/r/mode-javascript.js" and "/r/theme-twilight.js".

Good to know you can declare static files, although it would be nice if you could declare a static directory so you don't have to maintain configuration for every file that may be accessed after page load.

Let me know if you have any specific tasks or testing you need help with!

mizzao commented 10 years ago

Hi @swese44,

In the latest version of this package I have pulled in the entire ajaxorg/ace-builds repository as a submodule. This should make it easier pull in themes that you want. Do you want to try and see if you can get them working that way, and we can think about making this more robust for general use?

swese44 commented 10 years ago

Very cool, thanks for the update! I've updated Meteor to 0.8.0 with the latest version of this package, if I can get around the other error I'm having I'll let you know how it goes.

mizzao commented 10 years ago

Is this your site? (http://www.seemecode.com/). If you're developing that as a business, I'd love to see you come up with robust way of importing ace themes and modes for this package and contribute that back, since you're taking advantage of this Meteor integration already,

swese44 commented 10 years ago

It is my site, just a free service. If I get any time I will investigate. Thanks again for the work you've done!

gabrielhpugliese commented 10 years ago

I fixed it doing inside the config callback func:

      var config = require('ace/config');
      config.set('basePath', '/src-min');
      editor.setTheme('ace/theme/monokai');
      editor.getSession().setMode('ace/mode/javascript');

And put src-min folder from https://github.com/ajaxorg/ace-builds inside the /public folder of my Meteor project. Hope this works for you guys.

mizzao commented 10 years ago

Hi @gabrielhpugliese,

Thanks for that suggestion and digging up the basePath directive. I think to generalize this, we would add the style files in the package code with {isAsset: true} and set the basePath before the callback. This would make things much simpler for people who want to use this package. I'll try to integrate this when I get a chance.

EDIT: This seems like it would be helpful, if clunky: http://stackoverflow.com/a/20794116/586086

mizzao commented 10 years ago

Hey @swese44 and @gabrielhpugliese, this is included in 1d8137b59906ce210b2f5c015b7b142735bc2f4e now, released as 0.5.2. No need to copy up the source from the ace-builds folder yourself. You can just take that out and update the package. Also, just take out the config references as the path is set automatically on load.

Demo is updated to use the monokai theme: http://documents.meteor.com/

gabrielhpugliese commented 10 years ago

Awesome! Thank you!

gabrielhpugliese commented 10 years ago

I think the example has broken. If you don't take path on smart.json it works:

=> Errors prevented startup:

While building package `sharejs`:
fs.js:654:18: ENOENT, no such file or directory 'ace-builds/src'
  at Object.fs.readdirSync (fs.js:654:18)
  at walk (package.js:27:28)
  at getFilesFromFolder (package.js:50:16)
  at Object.use (package.js:74:21)
  at /home/gabra/.meteor/tools/6f23056589/tools/packages.js:1616:29
  at Array.forEach (native)
  at Function._.each._.forEach (/home/gabra/.meteor/tools/6f23056589/lib/node_modules/underscore/underscore.js:79:11)
  at _.extend.initFromPackageDir (/home/gabra/.meteor/tools/6f23056589/tools/packages.js:1420:7)
  at /home/gabra/.meteor/tools/6f23056589/tools/library.js:254:15
  at Object.enterJob (/home/gabra/.meteor/tools/6f23056589/tools/buildmessage.js:235:15)
  at _.extend.get (/home/gabra/.meteor/tools/6f23056589/tools/library.js:241:22)
  at /home/gabra/.meteor/tools/6f23056589/tools/packages.js:264:36
  at Array.forEach (native)
  at Function._.each._.forEach (/home/gabra/.meteor/tools/6f23056589/lib/node_modules/underscore/underscore.js:79:11)
  at /home/gabra/.meteor/tools/6f23056589/tools/packages.js:263:9
  at Array.forEach (native)
  at Function._.each._.forEach (/home/gabra/.meteor/tools/6f23056589/lib/node_modules/underscore/underscore.js:79:11)
  at _.extend.build (/home/gabra/.meteor/tools/6f23056589/tools/packages.js:261:7)
  at /home/gabra/.meteor/tools/6f23056589/tools/packages.js:1126:13
  at Array.forEach (native)
  at Function._.each._.forEach (/home/gabra/.meteor/tools/6f23056589/lib/node_modules/underscore/underscore.js:79:11)
  at _.extend.build (/home/gabra/.meteor/tools/6f23056589/tools/packages.js:1125:7)
  at _.extend.getForApp (/home/gabra/.meteor/tools/6f23056589/tools/library.js:285:9)
  at /home/gabra/.meteor/tools/6f23056589/tools/bundler.js:1720:25
  at Object.capture (/home/gabra/.meteor/tools/6f23056589/tools/buildmessage.js:191:5)
  at Object.exports.bundle (/home/gabra/.meteor/tools/6f23056589/tools/bundler.js:1655:31)
  at _.extend._runOnce (/home/gabra/.meteor/tools/6f23056589/tools/run-app.js:406:32)
  at _.extend._fiber (/home/gabra/.meteor/tools/6f23056589/tools/run-app.js:540:28)
  at /home/gabra/.meteor/tools/6f23056589/tools/run-app.js:348:12
mizzao commented 10 years ago

If you want the example to work, you will have to git clone --recursive the repo, or at least ensure that the Ace submodule is loaded.

Of course the example references the parent dir of the repo because it goes with the dev version. But if you are using this from Meteorite it will grab the git version and also check out the submodule for you.

I assure you it works, it's what I deployed to http://documents.meteor.com/

gabrielhpugliese commented 10 years ago

Yeah, I just realized that! :) Ty