extend event bus capabilities #417
Conversation
expose all lane actions to eventBus: - updateLanes - updateLane - moveLane - removeLane - addLane
the updateLane action already existed in code, but was not documented, fixed
LucasBadico
force-pushed
the
feat/extend-event-bus
branch
from
661ade5 to
1a2287f
Compare
| @@ -283,6 +283,19 @@ eventBus.publish({type: 'MOVE_CARD', fromLaneId: 'PLANNED', toLaneId: 'WIP', car | |||
| //To update the lanes | |||
| eventBus.publish({type: 'UPDATE_LANES', lanes: newLaneData}) | |||
|
|
|||
| //To update one lane | |||
| eventBus.publish({type: 'UPDATE_LANE', lane: { id: 'TODO', title: 'teste' }}) | |||
There was a problem hiding this comment.
this one already was in code, but was not documented.
| eventBus.publish({type: 'ADD_LANE', lane: { id: 'TODO', title: 'teste' }}) | ||
|
|
||
| //To move one lane | ||
| eventBus.publish({type: 'MOVE_LANE', fromIndex: 0, toIndex: 1 }) |
There was a problem hiding this comment.
I'm asking myself if it was not better to create a new action based on id, not on index. What do you think, @dapi?
There was a problem hiding this comment.
Hi @LucasBadico !
I agree with you. If we pass an id we can fetch lane's index when need.
What is you use case of MOVE_LANE event? Do you save lanes order to backend?
One thought came. When we drug one lane, from example move it from index 0 to index 5. Actually all lanes from 1 to 5 index are moved ) Shall we emit events for all these lanes? Probably better way to emit event like "LANES_MOVED" and pass new ordered lanes set?
There was a problem hiding this comment.
You have some points.
but my use case here:
- move lane was needed because the WebSocket and the feature of move lane and the socket keep the order of all clients synched.
- would be a good thing to have that Lanes Moved, but now I'm doing this on my backend, with a head and tail logic.
There was a problem hiding this comment.
Thanks @LucasBadico. Makes sense to expose other actions on the event bus
|
@LucasBadico do you mind resolving the merge conflict and I can add to coming release |
sure, will try to do this by end of my day. |
| case 'UPDATE_LANES': | ||
| return actions.updateLanes(event.lanes) | ||
| case 'UPDATE_LANE': | ||
| return actions.updateLane(event.lane) | ||
| case 'MOVE_LANE': | ||
| return actions.moveLane({ oldIndex: event.fromIndex, newIndex: event.toIndex }) |
There was a problem hiding this comment.
I know fromIndex and toIndexnames are better then oldIndex and newIndex. But it is preferable not to create new entities. We need either rename current attributes either use their. As we can't simply rename their because of using them by users we just have to use current names.
@LucasBadico What do you think?
There was a problem hiding this comment.
make sense. Principle of least surprise.
We needed more fine control over the board state through event bus.
Also added the doc for the new API.