geoext / geoext2

GeoExt 2 — JavaScript Toolkit for Rich Web Mapping Applications
http://geoext.github.io/geoext2/
Other
142 stars 106 forks source link

Introduce ExtJS 5 example app #352

Closed chrismayer closed 9 years ago

chrismayer commented 9 years ago

This PR introduces an ExtJS 5 based example app. The new example app is an ExtJS5-MVVM port of the existing MVC-example app. The port is not exactly one-by-one, but the output is very very similar to the MVC app.

The base structure has been created with Sencha Cmd and therefore it is compatible for the usage with Cmd. No compiled sources are included here because they can be easily generated with Sencha Cmd. So code redundancy is avoided.

At the moment the GeoExt package dependency is not resolved automatically. The developer has to clone the GeoExt project into the packages folder of the app on his local checkout of this example.

Steps after checking out this branch:

According to the discussion in #351 this seems not be final now, but I suggest to review this to have a start point we can work with.

marcjansen commented 9 years ago

After I checkout the branch, what steps am I supposed to do to see the example? Is tehre some sencha command that I need to run? I am a total noob with regard to sencha, sorry.

Other remarks from the top of my head:

marcjansen commented 9 years ago

I added a couple of other thoughts. Once I know how to look at this, I'll have another look. Thanks for the work so far on this, @chrismayer. I definitely want such an example in the library!

chrismayer commented 9 years ago

Hi @bentrm and @marcjansen, thanks for your reviews! I tried to adress your remarks (where possible). Would be cool, if you could have another look. I also added some steps to get the app running for dev purposes to the PR description.

  • Can we have it just like we had it with the old app-example, so that there is a index.html loading the compiled file and a index-dev.html loading loads of single files?

I am not sure if this is way to go with Sencha Cmd. It seems that you get your dev-setup by running sencha app watch, which starts you a webserver, which automatically restarts in case of changing the source files.

  • examples.json needs to have an entry for this example

I will go for this later on.

bentrm commented 9 years ago

I added some comments inline. Having a look at your work, I added support to build the app without recursively cloning GeoExt into the examples packages folder. Would you be ok with me sending a PR to your working branch?

marcjansen commented 9 years ago

Just cherry picking such a commit would probably be easier and would result in a nicer history. Just my two cents.

chrismayer commented 9 years ago

@bentrm thanks a lot for your ongoing effort. Feel free to modify the branch. As @marcjansen suggested maybe cherry-picking would be a good option.

marcjansen commented 9 years ago

WRT to cherry picking: simply publish your commits somewhere @bentrm, @chrismayer can then cherry-pick them if he likes them.

bentrm commented 9 years ago

Hi @chrismayer, @marcjansen, I just pushed to 4e9852de5091743c676bc7e972279ea165997baa. It should be possible to run sencha app watch and sencha app build inside of the app folder of the environment that you have described above without recursively cloning GeoExt into the example app folder.

I also removed the Sass stuff as it hasn't been used in the example app (setting skip.sass=1 in .sencha/app/sencha.cfg) and changed the build output directory to the GeoExt root folder (see app.json) adding temp file and directories to .gitignore.

marcjansen commented 9 years ago

But this isn't a commit that builds on top of Chris' work, is it?

bentrm commented 9 years ago

It is, it may have diverged though: branch log

marcjansen commented 9 years ago

So the right diff is chrismayer:ext5-app...bentrm:ext5-app.

@chrismayer Any chance you can check it out?

chrismayer commented 9 years ago

I merged the changes of @bentrm. @bentrm & @marcjansen can you have another look at this? TIA.

marcjansen commented 9 years ago

Good to be merged once the commit history is cleaned up. Thanks @chrismayer

chrismayer commented 9 years ago

I cleaned the commit history a bit. So now this is good to go, will merge now. Thanks to anyone who was involved.