Modified the store to handle conditionRelations#170
Conversation
|
Your package-lock.json is now exploded. You really like the Chinese npm. |
ugh! gross!! will remove asap. |
guergana
force-pushed
the
add-or-connection
branch
from
153bbf9 to
fcedbf8
Compare
guergana
requested review from
Ladsgroup and
bereket-WMDE
and removed request for
Ladsgroup
| }, | ||
| setConditionRelation( | ||
| context: ActionContext<RootState, RootState>, | ||
| payload: { value: ConditionRelation; conditionIndex: number } ): void { |
There was a problem hiding this comment.
I think this should also accept null.
| }, | ||
| addCondition( state: RootState ): void { | ||
| state.conditionRows.push( getFreshConditionRow() ); | ||
| if ( state.conditionRows.length === 0 ) { |
There was a problem hiding this comment.
This can be easily summarized:
state.conditionRows.push( getFreshConditionRow( state.conditionRows.length === 0 ) );
There was a problem hiding this comment.
I think that is not very easy to read, IMHO
There was a problem hiding this comment.
On second thoughts.. yes, it looks better.
| propertyValueRelation: PropertyValueRelation; | ||
| value: string | Value; | ||
| subclasses: boolean; | ||
| conditionRelation: ConditionRelation | null; |
There was a problem hiding this comment.
The logic in the code implies this condition relation is between this condition and the condition above (funnily enough, when implementing the component, I went with the exact opposite). This should be documented.
There was a problem hiding this comment.
You mean by adding a comment? Or documented how?
There was a problem hiding this comment.
Yes. Technically the relation doesn't belong to the query condition. It determines the relation between two. A comment in the QueryCondition would be good enough IMO
There was a problem hiding this comment.
I added some comments here
There was a problem hiding this comment.
Thanks. It's just it's below it and it should be usually above. Also maybe it should be in the RootState? here is mostly about sharing the link.
guergana
force-pushed
the
add-or-connection
branch
4 times, most recently
from
3701042 to
736794a
Compare
guergana
force-pushed
the
add-or-connection
branch
from
736794a to
f5f5add
Compare
|
Kudos, SonarCloud Quality Gate passed! |
2 participants
No description provided.