Merge pull request moodle#1 from NoelDeMartin/MOBILE-4239

MOBILE-4239: Coding style

docs/guides/templates/index.md

@@ -757,7 +757,7 @@ Let's assume that the context for this page doesn't match what the tabs template
 
 Unfortunately, we'll almost certainly never have complete control over all of the contexts that our template will be rendered in which means we'll be expecting people to write new webservices to supply the same data in different formats every time they want to use a template. It becomes an unmanageable problem.
 
-Enter blocks! We can make the template more flexible by defining sections of the template that can be overriden when they are included. Pretty neat! This will allow us to enforce a certain core structure but not enforce a context on the template that is including the tabs.
+Enter blocks! We can make the template more flexible by defining sections of the template that can be overridden when they are included. Pretty neat! This will allow us to enforce a certain core structure but not enforce a context on the template that is including the tabs.
 
 Let's have another go at that template, this time leveraging blocks:
 

general/app/development/development-guide.md

@@ -197,7 +197,7 @@ In addition to these files, feature folders may contain the following:
 - `directives/` and `pipes/` — Same as [core/directives/ and core/pipes/](#coredirectives-and-corepipes).
 - `lang.json` — See [Language files](#language-files).
 - `services/` — Same as [core/services/](#coreservices).
-- `pages/` — Page folders have the same structure as [core/components/](#corecomponents), but in addition they often declare modules as well to allow lazy-loading the page. In some cases, they can even have their own routing modules. For example, look at `core/features/mainmenu/pages/home/home-routing.module.ts`.
+- `pages/` — Page folders have the same structure as [core/components/](#corecomponents), but in addition they can declare modules if a page component is to be used in more than one module. Also, page components will declare their selectors starting with `page-`.
 
 In order to distinguish code from each feature, classes will be prefixed with the feature name. For example, the home page component declared in `core/features/mainmenu/pages/home/home.ts` is called `CoreMainMenuHomePage`.
 
@@ -266,7 +266,7 @@ Tests are found anywhere inside the `src/` folder, and they will be run as long
 Here are some examples:
 
 - The utils text service declared in `core/services/utils/text.ts` is tested in `core/services/tests/utils/text.test.ts`.
-- The init login page declared in `core/features/login/pages/init/init.ts` is tested in `core/features/login/tests/pages/init.test.ts`. The test file can be directly under `pages/` (instead of `pages/init/`) because the page component is the only file that will be tested for this folder. So it would be unnecessary to have a folder with a single file.
+- The credentials page declared in `core/features/login/pages/credentials/credentials.ts` is tested in `core/features/login/tests/credentials.test.ts`. The test file can be directly under `tests/` (instead of `tests/pages/credentials/`) because the page component is the only file that will be tested for this folder. So it would be unnecessary to have a folder with a single file.
 - The root app component declared in `app/app.component.ts` is tested in `app/app.component.test.ts`. The test file can live alongside the component because this module doesn't have any nested folders.
 
 In addition to unit test files, there is also a folder at `testing/` with setup and file utilities shared among all tests.
@@ -279,13 +279,11 @@ All core features and addons can define their own routes, and we can do that in
 
 With the [folders structure](#folders-structure) we're using, it is often the case where different core features or addons need to define routes depending on each other. For example, the *mainmenu* feature defines the layout and routes for the tabs that are always present at the bottom of the UI. But the home tab is defined in the *home* feature. In this scenario, it would be possible to just import the pages from the *home* module within the *mainmenu*, since both are core features and are allowed to know each other. But that approach can become messy, and what happens if an addon also needs to define a tab (like *privatefiles*)?
 
-As described in the [addons/ folder documentation](#addons), the answer to this situation is using the dependency inversion pattern. Instead of the *mainmenu* depending on anything rendering a tab (*home*, *privatefiles*, etc.), we can make those depend on the *mainmenu* instead. And we can do that using Angular's container.
+As described in the [addons/ folder documentation](#addons), the answer to this situation is using the dependency inversion pattern. Instead of the *mainmenu* depending on anything rendering a tab (*home*, *privatefiles*, etc.), we can make those depend on *mainmenu*. And we can do that using Angular's container.
 
 In order to allow injecting routes from other modules, we create a separated [Routing Module](https://angular.io/guide/module-types#routing-ngmodules). This is the only situation where we'll have a dedicated module for routing, in order to reduce the amount of module files in a feature root folder. Any routes that are not injected can be defined directly on their main or lazy module.
 
-It is often the case that modules using injected routes have a [RouterOutlet](https://angular.io/api/router/RouterOutlet). For that reason, injected routes can be defined either as children or siblings of the main route. The difference between those is that a child will be rendered within the outlet, whilst a sibling will replace the entire page. In order to make this distinction, routing modules accept either an array of routes to use as siblings or an object indicating both types of routes. You can see an example of this in the `core/features/courses/courses.module.ts` file where some *courses* routes are injected both as siblings and children into the home routing module.
-
-This architecture is not limited to feature or addon modules though, page modules can also define routing modules to allow injecting routes. One example is the `core/features/mainmenu/pages/home/home-routing.module.ts` file. Notice how the accompanying module does not use the *-lazy* name prefix, that is because page modules are loaded by their respective feature module and they are always lazy, so it isn't necessary to make that distinction (if a page is not lazy-loaded it doesn't have a dedicated module).
+It is often the case that modules using injected routes have a [RouterOutlet](https://angular.io/api/router/RouterOutlet). For that reason, injected routes can be defined either as children or siblings of the main route. The difference between those is that a child will be rendered within the outlet, whilst a sibling will replace the entire page. In order to make this distinction, routing modules accept either an array of routes to use as siblings or an object indicating both types of routes.
 
 ### Navigating between routes
 

general/development/policies/codingstyle-moodleapp.md

@@ -331,6 +331,82 @@ sayHello('World', undefined, undefined, 3);
 - Global declarations and extensions — Any variable you add to the window object can be declared by extending the `Window` interface. The same idea applies for extending external dependencies.
 - Local declarations — Sometimes, it may be useful to create a dedicated declaration file when source files are growing too large. But this technique should not be used to substitute proper code organisation.
 
+### Using constants
+
+Constants that are exported should be declared outside of a class, to favour tree shaking:
+
+<ValidExample title="Good">
+
+```ts
+export const MY_CONSTANT = '...';
+
+export class MyService {}
+```
+
+</ValidExample>
+
+<InvalidExample title="Bad">
+
+```ts
+export class MyService {
+
+    public static readonly MY_CONSTANT = '...';
+
+}
+```
+
+</InvalidExample>
+
+In contrast, constants that are private or protected should be declared as static readonly class properties. Also, avoid calling them using `this.CONSTANT` form (given that they are static members):
+
+<ValidExample title="Good">
+
+```ts
+export class MyService {
+
+    protected static readonly MY_CONSTANT = '...';
+
+    public someMethod(): void {
+        alert(MyService.MY_CONSTANT);
+    }
+
+}
+```
+
+</ValidExample>
+
+<InvalidExample title="Bad">
+
+```ts
+const MY_CONSTANT = '...';
+
+export class MyService {
+
+    public someMethod(): void {
+        alert(MY_CONSTANT);
+    }
+
+}
+```
+
+</InvalidExample>
+
+<InvalidExample title="Bad">
+
+```ts
+export class MyService {
+
+    protected static readonly MY_CONSTANT = '...';
+
+    public someMethod(): void {
+        alert(this.MY_CONSTANT);
+    }
+
+}
+```
+
+</InvalidExample>
+
 ## Angular
 
 ### Avoid calling methods in templates
@@ -465,3 +541,101 @@ export default class MyComponent {}
 ```
 
 </InvalidExample>
+
+### Declaring page modules
+
+When creating a page component, it should be declared in the feature's [lazy modules](../../../general/app/development/development-guide.md#routing). Exceptionally, pages that are used by more than one module can create a page module; but this module should only declare components, it shouldn't include any routing functionality.
+
+<ValidExample title="Good">
+
+```ts
+// file: core/features/feature/pages/index/index.ts
+@Component({
+    selector: 'page-core-feature-index',
+    templateUrl: 'index.html',
+})
+export class CoreFeatureIndexPageComponent {}
+```
+
+```ts
+// file: core/features/feature/feature-lazy.module.ts
+const routes: Routes = [
+    {
+        path: 'feature',
+        component: CoreFeatureIndexPageComponent,
+    },
+];
+
+@NgModule({
+    imports: [
+        RouterModule.forChild(routes),
+        CoreSharedModule,
+    ],
+    declarations: [
+        CoreFeatureIndexPageComponent,
+    ],
+})
+export class CoreFeatureLazyModule {}
+```
+
+</ValidExample>
+
+<CodeExample type="warning" title="Allowed only if the page is used in multiple modules">
+
+```ts
+// file: core/features/feature/pages/index/index.page.ts
+@Component({
+    selector: 'page-core-feature-index',
+    templateUrl: 'index.html',
+})
+export class CoreFeatureIndexPageComponent {}
+```
+
+```ts
+// file: core/features/feature/pages/index/index.module.ts
+@NgModule({
+    imports: [
+        CoreSharedModule,
+    ],
+    declarations: [
+        CoreFeatureIndexPageComponent,
+    ],
+})
+export class CoreFeatureIndexPageModule {}
+```
+
+</CodeExample>
+
+<InvalidExample title="Bad">
+
+```ts
+// file: core/features/feature/pages/index/index.page.ts
+@Component({
+    selector: 'page-core-feature-index',
+    templateUrl: 'index.html',
+})
+export class CoreFeatureIndexPageComponent {}
+```
+
+```ts
+// file: core/features/feature/pages/index/index.module.ts
+const routes: Routes = [
+    {
+        path: '',
+        component: CoreFeatureIndexPageComponent,
+    },
+];
+
+@NgModule({
+    imports: [
+        RouterModule.forChild(routes),
+        CoreSharedModule,
+    ],
+    declarations: [
+        CoreFeatureIndexPageComponent,
+    ],
+})
+export class CoreFeatureIndexPageModule {}
+```
+
+</InvalidExample>

versioned_docs/version-4.1/guides/templates/index.md

@@ -757,7 +757,7 @@ Let's assume that the context for this page doesn't match what the tabs template
 
 Unfortunately, we'll almost certainly never have complete control over all of the contexts that our template will be rendered in which means we'll be expecting people to write new webservices to supply the same data in different formats every time they want to use a template. It becomes an unmanageable problem.
 
-Enter blocks! We can make the template more flexible by defining sections of the template that can be overriden when they are included. Pretty neat! This will allow us to enforce a certain core structure but not enforce a context on the template that is including the tabs.
+Enter blocks! We can make the template more flexible by defining sections of the template that can be overridden when they are included. Pretty neat! This will allow us to enforce a certain core structure but not enforce a context on the template that is including the tabs.
 
 Let's have another go at that template, this time leveraging blocks: