saschakiefer / generator-openui5

yeoman generator for OpenUI5 applications and assets
Other
64 stars 17 forks source link

View and Controllers not placed in view directory #4

Closed js1972 closed 10 years ago

js1972 commented 10 years ago

The initial view and controller are in the root directory; but there is no sap.ui.localResources() call for this. Therefore the app attempts to read resources from the same location as the UI5 library which is ttps://sapui5.netweaver.ondemand.com/resources/ and fail.

If I move the view and controller into a view subdirectory and adjust the source code to suit; it works fine.

I'll fork the repo and try fix it. I'd like to add support for XML views as well... ;-)

saschakiefer commented 10 years ago

Hi,

I was able to reproduce the behavior you mentioned. It never hit me, cause I always put my views in a sub directory (therefore the proposed default view is "view.Main" -> Main.js and Main.controller are created in sub directory view). Just to make sure: You created a view without a prefix (like "Main" without "view." prefixed), right?

Having that said, that leads me to another flaw I recognized: The default view directory is "view". For this, there is also a sap.ui.localResources("view"); statement generated in index.html. If a user now create a view in a different directory, which is not made known to the framework, it will always run into an issue. Therefore a new feature would be to check during the view generation, if there is an related localResources definition available, if not, generate one. I'll track that as errors/ feature request. Unfortunately I'm currently a bit short on time...

XML View Support would be highly appreciated ;-)

js1972 commented 10 years ago

Will take a look at making some changes next week. I've gotta work out how these yo generators work first as I've never built one before...

On Thu, Nov 21, 2013 at 5:06 PM, saschakiefer notifications@github.com wrote:

Hi, I was able to reproduce the behavior you mentioned. It never hit me, cause I always put my views in a sub directory (therefore the proposed default view is "view.Main" -> Main.js and Main.controller are created in sub directory view). Having that said, that leads me to another flaw I recognized: The default view directory is "view". For this, there is also a sap.ui.localResources("view"); statement generated in index.html. If a user now create a view in a different directory, which is not made known to the framework, it will always run into an issue. Therefore a new feature would be to check during the view generation, if there is an related localResources definition available, if not, generate one. I'll track that as errors/ feature request. Unfortunately I'm currently a bit short on time...

XML View Support would be highly appreciated ;-)

Reply to this email directly or view it on GitHub: https://github.com/saschakiefer/generator-sapui5/issues/4#issuecomment-28967276

saschakiefer commented 10 years ago

I created a new issue for the XML View and added you as an owner. Hope, that's OK ;) I also created a new issue for the declaration of new sub directors if they are not yet present in application.js

saschakiefer commented 10 years ago

I think I found a small bug in your implementation. Instead of just checking for view.Main in

this.viewName = (props.viewName === 'view.Main')? props.viewName : 'view.' + props.viewName;

https://github.com/saschakiefer/generator-openui5/blob/master/view/index.js#L41

we should check, if the name entered start's with view. in that case adding the view prefix should be avoided. I think that makes it more error save. Otherwise the user might ask, why view.Main works and view.Test becomes view.view.Test

js1972 commented 10 years ago

Yes I knew about that one and you're right. However I was thinking from the perspective of "convention over configuration" so that ALL views go into the view folder regardless - that's the convention. What I should have done is blocked users from naming a view starting with "view.".

What do you think??

I might get a but unwieldy to allow every possible combination and permutation and maybe better to follow the convention over configuration route that allot of frameworks follow. 

Up to you.  ;-)

I'm on Xmas leave at the moment but will be back to work on XML views and anything else on the 3rd of jan.

On Mon, Dec 23, 2013 at 12:46 AM, saschakiefer notifications@github.com wrote:

I think I found a small bug in your implementation. Instead of just checking for view.Main in

this.viewName = (props.viewName === 'view.Main')? props.viewName : 'view.' + props.viewName;

https://github.com/saschakiefer/generator-openui5/blob/master/view/index.js#L41

we should check, if the name entered start's with view. in that case adding the view prefix should be avoided. I think that makes it more error save. Otherwise the user might ask, why view.Main works and view.Test becomes view.view.Test

Reply to this email directly or view it on GitHub: https://github.com/saschakiefer/generator-openui5/issues/4#issuecomment-31090795

saschakiefer commented 10 years ago

That's a fair point. Especially, when I think about supporting all possible combinations. What I'm going to do then is, removing the view. part of the default view name and just default it to Main. I think that makes it more consistent then.

I just don't wan't to push convention over configuration too far, cause that one thing that really annoys me when using maven ;-)