Details
Please see the video to see how the adding calendar event interaction now works with this new component.
To test interaction on local machine:
Replace this.setState({ pickerOpen: true }); on line 213 of web/calendar/day.react.js
this.props.pushModal( <ThreadPickerModal name="Chats" onClose={this.props.popModal} createNewEntry={this.createNewEntry} />, );
import ThreadPickerModal with import ThreadPickerModal from '../modals/threads/thread-picker-modal.react';
And add the popModal function by adding this code in the ConnectedDay component and Day component props:
Diff Detail
- Repository
- rCOMM Comm
- Branch
- eng-1778 (branched from master)
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Looks good! Minor nits inline
web/modals/threads/thread-picker-modal.css | ||
---|---|---|
25 | nit: We call it "splotch" elsewhere in the codebase: so we could probably rename this to threadSplotch? | |
web/modals/threads/thread-picker-modal.react.js | ||
24 | Can we name this something a little more specific than onClick to make the purpose of this function a bit clearer? | |
25–26 | Thoughts on swapping these two function calls positionally? | |
29–34 | Nice, good catch memoizing this style object! (Same note as above but something like splotchColorStyle would be more consistent with what's already in the codebase) | |
56–98 | Thanks for linking the diff where this logic was previously reviewed/approved | |
112 | Good call displaying "No results" if there aren't any results... makes it clear to the user that we're not waiting to load them or anything |
web/modals/threads/thread-picker-modal.react.js | ||
---|---|---|
79 ↗ | (On Diff #17426) | Defining this createSelector inline in a function component makes zero sense. The whole point of createSelector is its caching behavior... but this createSelector gets redefined on every render, so no caching is achieved |
93–99 ↗ | (On Diff #17426) |