feat: Story map chapter move #1175
Conversation
josebui
changed the title
|
|
||
| const onClick = useCallback(event => { | ||
| setOpenConfirmation(true); | ||
| event.stopPropagation(); |
There was a problem hiding this comment.
question: I'm not super familiar with the web client code, is the event.stopPropagation used commonly throughout, or is there a special reason you are using it here?
There was a problem hiding this comment.
@david-code yes, I think that is common, there are some cases where you have the same listener on a parent component and you want to avoid the event to keep propagating
| props; | ||
|
|
||
| useEffect(() => { | ||
| setOpenConfirmation(false); |
There was a problem hiding this comment.
suggestion+nit: I don't think this needs to be set false here, I think that is the default value it will have.
| const withoutChapter = config.chapters.filter( | ||
| chapter => chapter.id !== id | ||
| ); | ||
| const newChapters = [ | ||
| ..._.slice(0, index, withoutChapter), | ||
| config.chapters.find(chapter => chapter.id === id), | ||
| ..._.slice(index, withoutChapter.length, withoutChapter), | ||
| ]; | ||
| return newChapters; |
There was a problem hiding this comment.
suggestion(not neccessary): I think if this is used elsewhere in the code, it could be a good idea to make a separate function to encapsulate the immutable replace.
There was a problem hiding this comment.
comment: I don't have enough experience with this part of the web client to give a good review of the tests themselves here, but they look nice to me at a quick glance.
Description
Checklist
Related Issues
Fixes #1108