-
Notifications
You must be signed in to change notification settings - Fork 2
WIP alternative hierarchic config #255
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
base: master
Are you sure you want to change the base?
Conversation
| public readonly itemNode = nodeConfig({ | ||
| service: ItemService, | ||
| root: true, | ||
| selectableAtKey: 'any', | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
D'abord on définit les noeuds de l'arbre. La nouvelle propriété root permet de spécifier le(s) root(s) du tree.
| public hierarchicConfig = hierarchicConfig( | ||
| [this.itemNode], | ||
| [ | ||
| { | ||
| parent: this.itemNode, | ||
| child: this.itemNode, | ||
| field: 'parent', | ||
| }, | ||
| ], | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Puis on définit les relations entre les noeuds de l'arbre. Une relation est un parent et un child, lié par un field (comme avant, ce field est utilisé pour fabriqué le filtre pour obtenir les enfants à partir du parent).
| root: { | ||
| filter: { | ||
| conditions: [ | ||
| // ... | ||
| ], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dans cet autre exemple, on override le filtre par défaut pour obtenir les roots ({empty: {}}) avec un filtre custom. Permettant donc d'avoir des racines différents en fonctions des besoins métiers.
Cela devrait permettre de résoudre #11914.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En fait je crois plus que cela soit une bonne idée. Parce qu'il est préférable de modifier le comportement {empty: {}} directement du côté du serveur, pour garantir une cohérence partout.
| public hierarchicConfig2 = hierarchicConfig( | ||
| [this.itemForRootNode, this.itemForChildNode], | ||
| [ | ||
| { | ||
| parent: this.itemForRootNode, | ||
| child: this.itemForChildNode, | ||
| field: 'parent', | ||
| }, | ||
| { | ||
| parent: this.itemForChildNode, | ||
| child: this.itemForChildNode, | ||
| field: 'parent', | ||
| }, | ||
| ], | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Les roots sont filtrées de façon spéciales, mais tous les enfants, récursivement, n'ont plus de filtre spécial. En fait non.
| export function nodeConfig<T extends UntypedModelService>(node: NodeConfig<T>): NodeConfig<T> { | ||
| return node; | ||
| } | ||
|
|
||
| export function hierarchicConfig<Nodes extends NodeConfig[]>( | ||
| nodes: Nodes, | ||
| relations: RelationConfig<Nodes>[], | ||
| ): NaturalHierarchicConfiguration<Nodes> { | ||
| return {nodes: nodes, relations: relations}; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On utilise 2 fonctions pour construire la config pour permettre à TypeScript de nous garantir que les noeuds utilisés dans les relations, sont bien déclarés dans la liste de noeuds.
|
Tout ce qui m'importe c'est qu'il n'y ai pas de régressions. La syntaxe m'importe peu, sens toi libre de changer à ton goût. |
Proposition d'alternative pour la config du sélecteur hiérarchique. L'idée principale est de d'abord déclarer les noeuds, puis de déclarer les relations possibles entre chaque pair de noeuds. Ainsi on a plus besoin de l'ancien
childrenRelationNamesqui pouvait être invalide de façon complètement silencieuse (et était non-typé). J'ai l'impression que cette structure est plus facile à suivre par l'humain.En plus de cela, on ajoute la nouvelle possibilité d'overrider le filtre utilisé pour obtenir les roots. Cette partie pourrait être faite sans le reste du refactor, pour résoudre #11914.En fait non, je crois pas que c'est nécessaire du tout.@sambaptista, @stissot est-ce que cette nouvelle façon de faire, illustrée via https://github.com/Ecodev/natural/pull/255/files#r2488822913, vous semble aller dans la bonne direction ?