Typo and minor code change

[nicoschoenteich]

Thanks a lot for the fantastic tutorial 🙌🏻

[akudev]

@nicogeburek Thanks for the PR! The typo is clear and I would merge it immediately. However, the code change leaves me a bit puzzled: which "docs" are you referring to as "suggesting" the change?

The tutorial documentation itself says in this section: "...and sets the model to the view...". When setting it on the component, at least this explanation would also have to be changed.

Furthermore, there is the full application code for each exercise, the respective line is at https://github.com/SAP-samples/ui5-typescript-tutorial/blob/main/exercises/ex6/com.myorg.myapp/src/controller/IncidenceDetail.controller.ts#L16, but also in the folders "ex7", "ex8",... for the subsequent exercises. Plus, in the gh-pages branch there are two copies of the app for each exercise plus for the backup data version. So when you change the code snippet in the documentation, you also need to adapt those about eleven other places and the documentation text.

However, as I just noticed, the code in all those app copies is already out of sync with the documentation: it says

this.setModel(model, "incidenceHistory");

• making use of the "setModel" method in the recently added base controller.

So my suggestion would be to not replace getView() with getOwnerComponent() but to just remove it.

1. it is the smallest change to bring code and tutorial in sync again
2. it makes use of the base controller, demonstrating its purpose and reducing code size
3. IMHO setting this model loaded by the detail view only on the detail view makes more sense than setting it on the entire component: a model on the component whose existence depends on whether the user has already navigated to a detail view sounds strange and may cause issues when used elsewhere.

The only thing I would do on top is mentioning the base controller, e.g. by saying "...and sets the model to the view, using the setModel method inherited from the BaseController. " (emphasis just to show what's new)

What do you think? Would you amend this PR or prefer me to do it?

Cheers Andreas

[nicoschoenteich]

Hi @akudev, I guess "docs" was a little unspecific 😅 I found the note in line 26532 of sap.ui.core.d.ts (link to raw file, too large to be rendered in GH) where the onInit() method of the Controller is defined.

I think all of your suggestions work, so let's go with the one that requires least effort 😉 BR, Nico

[akudev]

@nicogeburek I see. But that's the opposite case: when the controller wants to get access to a model which is assigned to a component (that's the precondition; often models are defined in the manifest and end up on the respective component, so this situation occurs often), then the controller should not fetch the model from its view, but directly from the component, because otherwise there's the risk that component and view are not connected yet and the model set on the component has not yet cascaded down to the view.

Here we are in the situation where we want to set a model to make use of it in the view. True, if the model would be needed in sibling views, then we would have to set it on the component. But less because of this lifecycle issue above, but more because the model would not be available elsewhere. But we don't need that. And setting the model on the controller's view is not impacted at all by the question whether component and view are already hierarchically connected.

Regarding the change: would you do that? (just make it this.setModel(model, "incidenceHistory");)

[nicoschoenteich]

Oh, that makes sense. I definitely misinterpreted the documentation. Thanks a lot for the explanation.

I will do the suggested changes and update this PR 👍🏻

[nicoschoenteich]

Hi @akudev, done 👍🏻

[akudev]

Thanks!