Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(components): update documentation on components #1583

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthieu-crouzet
Copy link
Contributor

Proposed change

Update documentation for components

Copy link

nx-cloud bot commented Apr 3, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 8e0a717. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@github-actions github-actions bot added documentation Improvements or additions to documentation project:@o3r/core labels Apr 3, 2024
@matthieu-crouzet matthieu-crouzet marked this pull request as ready for review April 4, 2024 08:21
@matthieu-crouzet matthieu-crouzet requested a review from a team as a code owner April 4, 2024 08:21
docs/components/COMPONENT_REPLACEMENT.md Outdated Show resolved Hide resolved
docs/components/COMPONENT_REPLACEMENT.md Outdated Show resolved Hide resolved
docs/components/COMPONENT_REPLACEMENT.md Outdated Show resolved Hide resolved
docs/components/COMPONENT_REPLACEMENT.md Outdated Show resolved Hide resolved
docs/components/COMPONENT_REPLACEMENT.md Outdated Show resolved Hide resolved
docs/localization/LOCALIZATION.md Outdated Show resolved Hide resolved
docs/styling/THEME.md Outdated Show resolved Hide resolved
docs/styling/THEME.md Outdated Show resolved Hide resolved
docs/styling/THEME.md Outdated Show resolved Hide resolved
docs/styling/THEME.md Outdated Show resolved Hide resolved
docs/components/COMPONENT_REPLACEMENT.md Outdated Show resolved Hide resolved
docs/components/COMPONENT_REPLACEMENT.md Outdated Show resolved Hide resolved
docs/components/INTRODUCTION.md Outdated Show resolved Hide resolved
sdo-1A
sdo-1A previously approved these changes Apr 4, 2024
docs/configuration/CONFIGURATION_SUPPORTED_EXTRACTOR.md Outdated Show resolved Hide resolved
docs/configuration/CONFIGURATION_SUPPORTED_EXTRACTOR.md Outdated Show resolved Hide resolved
docs/configuration/CONFIGURATION_SUPPORTED_EXTRACTOR.md Outdated Show resolved Hide resolved
docs/configuration/CONFIGURATION_SUPPORTED_EXTRACTOR.md Outdated Show resolved Hide resolved
docs/configuration/OVERVIEW.md Outdated Show resolved Hide resolved
docs/configuration/OVERVIEW.md Outdated Show resolved Hide resolved
docs/configuration/OVERVIEW.md Outdated Show resolved Hide resolved
docs/configuration/OVERVIEW.md Outdated Show resolved Hide resolved
docs/configuration/OVERVIEW.md Outdated Show resolved Hide resolved
docs/configuration/OVERVIEW.md Outdated Show resolved Hide resolved
````

Are not possible through an ``ng-template`` and ``c11n`` combination.

Though there is a solution for the first example in making the value an input, and bind it inside the component using
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make it clear that the value we are talking about is the dynamicClass? I know that is the only value on the container, but using precise words may avoid confusion.

````

Are not possible through an ``ng-template`` and ``c11n`` combination.

Though there is a solution for the first example in making the value an input, and bind it inside the component using
the [HostBinding](https://angular.io/api/core/HostBinding) decorator, there is no actual solution for applying directive.
A [feature request](https://github.com/angular/angular/issues/8785) has been opened for a long time and finally made it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the feature request finally made it, what is the next step ? :D

Configration example:

Configration example:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Configration example:
Configuration example:

import {Context} from '@o3r/core';

export interface DummyPresContextInput {
dummyInput: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a dummy doc ( as line 188 says) :)
same for output.


We prefer to use the __formControl__ rather than __ngModel__ because we can easily listen to the valueChanges or status changes of the presenter form.
Another constraint is that it's easier to identify the container context for the CMS, with one implementation (See [Component Structure](../components/COMPONENT_STRUCTURE.md) for details about the component context).
Another constraint is that it's easier to identify the container context for the CMS, with one implementation (See [Component Replacement](../components/COMPONENT_REPLACEMENT.md) for details about the component context).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean "with one implementation"?

Comment on lines 93 to 94
In addition to the simple case, if we need an __error message__ panel, which can be displayed anywhere in the page,
or we need __form submission__, done from the page, we came up with the following implementation.
or we need __form submission__, done from the page, we came up with the following implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In addition to the simple case, if we need an __error message__ panel, which can be displayed anywhere in the page,
or we need __form submission__, done from the page, we came up with the following implementation.
or we need __form submission__, done from the page, we came up with the following implementation.
In addition to the simple case, if we need an __error message__ panel, which can be displayed anywhere in the page, or to __submit the form outside the component__, we follow the more complex implementation described below.

##### 1.Basic structure
The form created in the presenter and the default value should have the same contract. The contract of a form is an interface which defines the form controls names and the type of the value which should be handled by each control. See the example of a component creation.
##### 1.Basic structure
The form created in the presenter and the default value should have the same contract. The contract of a form is an interface which defines the form controls names and the type of the value which should be handled by each control. See the example of a component creation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The form created in the presenter and the default value should have the same contract. The contract of a form is an interface which defines the form controls names and the type of the value which should be handled by each control. See the example of a component creation.
The form created in the presenter and the default value should have the same contract. The contract of a form is an interface which defines the name of the form controls and the type of the value which should be handled by each control. See the example of a component creation.

@@ -1,6 +1,6 @@
# Container / Presenter

We encourage developers to decouple components into containers and presenters. From a UI perspective it is a good practice to separate access of Data/ business logic form pure presentation, this allows developer to reuse presenters in other parts of the code with different data or having a container linked to multiple presenters, in case you want to display the same thing with a totally different user experience.
We encourage developers to decouple components into containers and presenters. From a UI perspective it is a good practice to separate access of data/business logic form pure presentation. This allows developers to reuse presenters in other parts of the code with different data or having a container linked to multiple presenters, in case you want to display the same thing with a totally different user experience.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"We encourage developers to decouple components into containers and presenters." Not sure it's relevant for all the use cases, if you are building a library or if you want to split the work between container and presenters amongst developers, or for a product that is meant to be configured and customized yes, but that's about it I think.

export class ComponentNameComponent {}
```
The object passed to the `@O3rComponent` decorator includes the component type, which can be:
- `Block`: a component that handles a functional area
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an example here


First thing to do is to define your given filenames for the configuration in the _package.json_ of the library/app where you run the extractor.
When running in a library it will use this configuration as the names for the metadata files.
When running the extractor in an application, it will search for these filenames in each node_module (package.json file) of each library,
in order to concat the metadata from the file with other libraries metadata and app metadata.
in order to concat the file's metadata with the metadata of other libraries and the application's metadata.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final metadata file generated for an application will be the concatenation of the ones provided by the library used (if available) and the one extracted from the application itself.

@@ -15,25 +15,25 @@
3. [Include Custom Validations](#custom-validators)
1. [Validators definition](#custom-validators-definition)
2. [Apply validators ](#custom-apply-validators)
3. [Validators translations](#custom-validators-translations)
3. [Validators translations](#custom-validators-translations)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Structure is too complex for one page and causes issues in the structure

* __submit from presenter__ - the submit button is displayed
* __Presenter__ - click on submit btn and emits an event > __Container__ receives the signal and executes the submit logic. Emits an event when the submit logic is finished.

* __Presenter__ - click on submit btn and emits an event > __Container__ receives the signal and executes the submit logic. Emits an event when the submit logic is finished.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* __Presenter__ - click on submit btn and emits an event > __Container__ receives the signal and executes the submit logic. Emits an event when the submit logic is finished.
* __Presenter__ - click on submit button and emits an event > __Container__ receives the signal and executes the submit logic. Emits an event when the submit logic is finished.

@@ -3,7 +3,7 @@

Localization module is built on top of an open source [ngx-translate](https://github.com/ngx-translate/core) library.
Copy link
Contributor

@cpaulve-1A cpaulve-1A Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Localization module is built on top of an open source [ngx-translate](https://github.com/ngx-translate/core) library.
The ``Localization`` module is built on top of an open source [ngx-translate](https://github.com/ngx-translate/core) library.

@@ -29,7 +29,7 @@ Localization module is built on top of an open source [ngx-translate](https://gi

# How to use

We provide in [library](https://github.com/AmadeusITGroup/otter/blob/main/packages/@o3r/localization/src/tools/localization.module.ts) an angular module called **LocalizationModule** which comes with translations loader.
We provide in [library](https://github.com/AmadeusITGroup/otter/blob/main/packages/@o3r/localization/src/tools/localization.module.ts) an Angular module called **LocalizationModule** which comes with translations loader.

- **In your AppModule** you need to **import** the **LocalizationModule** and **TranslateModule**. The LocalizationModule could be imported calling `forRoot` with a custom configuration **factory** to specify the language of the application. This configuration is of type **LocalizationConfiguration** and describes your endpoint URL, supported locales, list of RTL languages, the language of your application and your fallback language.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- **In your AppModule** you need to **import** the **LocalizationModule** and **TranslateModule**. The LocalizationModule could be imported calling `forRoot` with a custom configuration **factory** to specify the language of the application. This configuration is of type **LocalizationConfiguration** and describes your endpoint URL, supported locales, list of RTL languages, the language of your application and your fallback language.
- **In your AppModule** you need to **import** the ``LocalizationModule`` and ``TranslateModule``. The LocalizationModule could be imported calling `forRoot` with a custom configuration **factory** to specify the language of the application. This configuration is of type ``LocalizationConfiguration`` and describes your endpoint URL, supported locales, list of RTL languages, the language of your application and your fallback language.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method names and functions should be between `` shouldn't they?
(Comment for other parts of the file)

### How to localize a date, decimal and currency

Use angular built-in [DatePipe](https://angular.io/api/common/DatePipe), [DecimalPipe](https://angular.io/api/common/DecimalPipe) and [CurrencyPipe](https://angular.io/api/common/CurrencyPipe) and pass it current locale as the last parameter. The locale is read from **this.localizationService.getCurrentLanguage()**. To be able to use translateService, your component container should take benefit of dependency injection to get LocalizationService as parameter of constructor as well.
Use Angular built-in [DatePipe](https://angular.io/api/common/DatePipe), [DecimalPipe](https://angular.io/api/common/DecimalPipe) and [CurrencyPipe](https://angular.io/api/common/CurrencyPipe) and pass it current locale as the last parameter. The locale is read from **this.localizationService.getCurrentLanguage()**. To be able to use translateService, your component container should take benefit of dependency injection to get LocalizationService as parameter of constructor as well.

For example if you want to localize simpleHeader component you will start by injecting TranslateService to the constructor of simple-header-pres.component.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For example if you want to localize simpleHeader component you will start by injecting TranslateService to the constructor of simple-header-pres.component.ts
For example if you want to localize the SimpleHeader component you will start by injecting TranslateService to the constructor of simple-header-pres.component.ts


### Generate the RefX theme variables
### Generate your own theme variables
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: there are other reference to refx and airlines in the examples and in this documentation.

@@ -242,11 +239,11 @@ to style a material component with a new palette (e.g. highlight), you will need

#### Process

The process to customize the application theme is similar to the angular material one as describe in [Theming your
The process to customize the application theme is similar to the Angular material one as describe in [Theming your
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 232: the four palettes material palettes - leftover duplicate

@@ -1,31 +1,31 @@
# Configuration mechanism

The aim of this document is to help developers to implement configuration inside components, in order to be compliant
with __cms architecture__. This will give the possibility to __Business Analyst__ to configure the
The aim of this document is to help developers to implement `Configuration` inside components, in order to be compliant with the __CMS architecture__.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the first thing is not about the CMS, it's about making the application configurable dynamically. The CMS is a way to ease the creation of this configuration override, but that's not mandatory. It would be interesting to develop "When do I want to build configurability in my application and why" and then go on the How which is already part of this file

- Defined in one interface extending the __AppBuildConfiguration__ from __@o3r/core__ in order
to be identified by the extractor
- PreBootstrap config object is used for configurations needed before loading the Angular app component, and it will be
injected by the CMS in the index.html body tag data-bootstrapconfig as a data attribute
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not injected by the CMS, there is no runtime interactions with the CMS, it's used as an edition panel for the customization and the files are exported in the dynamic content folder afterwards. We should advertise that this part is linked to a piece of code server side that updates the index.html and what requires to be added with an example

docs/components/INTRODUCTION.md Outdated Show resolved Hide resolved
To enable the communication between your application and the Chrome Extension you can follow this [documentation](../dev-tools/chrome-devtools.md).

## Next steps

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This session is missing references to features configuration, localization, styling and design that can affect the component architecture in term of files.

You should also provide a link to placeholder that is part of the component page.

You may also need to add a reference list to the module affecting directly the component code: (today only the eslint plugin/config would be part of this list)

| **Template file name** | *-cont.template.html |
| **Unit test file name** | *-cont.spec.ts |

It has its own _index.ts_ file exporting:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure you need to remove this part as the bundle still exist to export modules and component

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is pertinent in the documentation ?
What does it bring to the developer that we write it here ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that we export the different file of the components in a barrel, that does not include all of then per default, soon to be an information that is meaningful.

docs/components/CONTAINER_PRESENTER.md Show resolved Hide resolved
container/
presenter/
elements/
my-element/
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A section with links/references to the features extending the component capabilities should be provided to this file (equivalent to the next steps section from the introduction file)

docs/components/INTRODUCTION.md Outdated Show resolved Hide resolved
docs/components/CONTAINER_PRESENTER.md Show resolved Hide resolved
```json
"cmsMetadata": {
"componentFilePath": "./component.class.metadata.json",
"configurationFilePath": "./component.config.metadata.json",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"configurationFilePath": "./component.config.metadata.json",
"configurationFilePath": "./component.config.metadata.json"

@@ -52,13 +52,13 @@ In angular.json file of your lib.
"builder": "@o3r/core:lib-build", // wrapper builder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

turn json to json5 to support comment in the rendered code block

@@ -1,9 +1,9 @@
# Tests setup

With the focus on simplicity, we chose to use [JEST](https://jestjs.io) as Testing Framework for our unit and integration tests, which aims to work with a minimum set of configurations, on most js projects.
With the focus on simplicity, we chose to use [JEST](https://jestjs.io) as Testing Framework for our unit and integration tests, which aims to work with a minimum set of configurations, on most js projects.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
With the focus on simplicity, we chose to use [JEST](https://jestjs.io) as Testing Framework for our unit and integration tests, which aims to work with a minimum set of configurations, on most js projects.
With the focus on simplicity, we chose to use [Jest](https://jestjs.io) as Testing Framework for our unit and integration tests, which aims to work with a minimum set of configurations, on most js projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants