search

saschakiefer / generator-openui5

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

Navigation via EventBus #50

Closed js1972 closed 10 years ago

js1972 commented 10 years ago

The best practices described by SAP (in the SDK) state that its wise to use the Event Bus for navigation. Our apps are based around the TechEd Fiori example which do not... Should we enhance (in the future) for the Event Bus?

js1972 commented 10 years ago

fyi. We currently set a property on each Pages controller which points to the root controller, such that the root controller controls all navigation. This is setup in the root controller by iterating over the Pages aggregation. So... this is still pretty-much decoupled and no more difficult that using the EventBus...

js1972 commented 10 years ago

Interesting.... the sap.m.SplitContainer's (used by sap.m.SplitApp) undocumented to() method which is being used for forward navigation, internally uses the EventBus. Hmmm... It does use the event bus under the hood but not sure that I like the architecture of binding each view to the root views controller to control nav. I'm thinking that using the event bus as detailed in the sap.m best practices in the SDK looks nicer and is easier to understand whats going on.

See examples in the SDK of event bus - based navigation here: https://openui5.hana.ondemand.com/#docs/guide/MVC.1.html.

js1972 commented 10 years ago

More doco from the sdk here: https://openui5.hana.ondemand.com/#docs/guide/Navigation.2.html.

Question: Which way is better:

(a) Couple the detail views to the root view via a property and use the sap.m.SplitApp's to() methods; or,

(b) Use the EventBus as per the SDK.

saschakiefer commented 10 years ago

I really don't know which one is better in the end. My gut feeling, when I saw a) in the coding was: I don't like it much (without being able to put my finger on it)

js1972 commented 10 years ago

Well I thought (b) is better as well which is why I sneaked in the above pull-request. This will close if you want to merge the pull-request...

js1972 commented 10 years ago

Closing... I'm sure this issue will pop up again as UI5 evolves...