TreeCollection & RuleCollection (Conditional Tree Builder) #456
Conversation
…-spfxcontrolsreact
There was a problem hiding this comment.
@edarroudi - thank you very much for this PR.
And I'm really sorry for the long delay.
I left some comments.
Promise to be more responsive :)
| @@ -29,6 +29,7 @@ | |||
| "@pnp/sp": "1.3.11", | |||
| "@pnp/sp-clientsvc": "1.3.11", | |||
| "@pnp/sp-taxonomy": "1.3.11", | |||
| "@pnp/spfx-controls-react": "3.8.0", | |||
There was a problem hiding this comment.
Did you check what is the size increase for the library?
Maybe it makes sense (if needed) to incorporate dynamic import?
| @@ -0,0 +1,15 @@ | |||
| import { IPropertyFieldRuleTreeProps as IPropertyFieldRuleTreeProps } from '.'; | |||
There was a problem hiding this comment.
It's better to import from the file instead of ..
When importing from . you may complicate tree shaking.
| } | ||
|
|
||
|
|
||
| private readonly standardFields: ICustomCollectionField[] = [{ |
There was a problem hiding this comment.
Definitely needs to be localized.
| names.push(<li>{objName} {this.getTokens(tokenObject[objName])}</li>); | ||
| } | ||
|
|
||
| if(names) |
There was a problem hiding this comment.
this will be always true.
You should probably check names.length
| import { ICustomCollectionField } from "../collectionData/ICustomCollectionField"; | ||
| import { ICustomTreeData, ICustomTreeItem } from "./ICustomTreeItem"; | ||
|
|
||
| export interface IPropertyFieldTreeCollectionDataProps { |
There was a problem hiding this comment.
this interface seems pretty similar to IPropertyFieldRuleTreeProps.
Maybe it makes sense to extend one from another or make some basic interface that will be extended by both?
|
|
||
| import { clone, findIndex, sortBy } from '@microsoft/sp-lodash-subset'; | ||
| import { CollectionDropdownField } from '../../collectionData/collectionDropdownField/CollectionDropdownField'; | ||
| import { isEqual } from 'lodash'; |
There was a problem hiding this comment.
import isEqual from 'lodash/isEqual'
| import { CollectionColorField } from '../../collectionData/collectionColorField/CollectionColorField'; | ||
| import { CollectionIconField } from '../../collectionData/collectionIconField/CollectionIconField'; | ||
|
|
||
| export class TreeCollectionDataItem extends React.Component<ITreeCollectionDataItemProps, ITreeCollectionDataItemState> { |
There was a problem hiding this comment.
I don't like duplicating of the code between this component and CollectionDataItem.
Maybe implement some helper that will contain common logic?
Or have a base component that will have props like render, etc. And both TreeCollectionDataItem and CollectionDataItem will render this base component with appropriate props overrides?
@AJIXuMuK - no worries! Thank your for your reply and your comments. I tend to optimize and refactor the code once the overall "Architecture", Layout and Features are discussed and approved. I believe now that you don't have any problems with the overall approach and will a) change the code according to your commets and b) add the Evaluation and Custom conditions code. |
What's in this Pull Request?
Added a common TreeCollection control that features:
Added a RuleTree (Condition Builder Tree) based on the TreeCollection component above, featuring:
What are the next todos for this feature branch:
What are the optional todos for the next feature branch:
Any comment is appreciated. I think the trees don't need to support a lot of items so I didn't optimize it for speed.
The RuleBuilder might look a bit confusing at first but I think users will be able to work with that.
Below are some screenshots...