Skip to content

Conversation

@OSPFNeighbour
Copy link
Contributor

Moved tag selector from groups page into a component.

@codecov
Copy link

codecov bot commented Jul 17, 2018

Codecov Report

Merging #543 into master will increase coverage by 0.58%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #543      +/-   ##
==========================================
+ Coverage   26.69%   27.27%   +0.58%     
==========================================
  Files         189      189              
  Lines        3136     3120      -16     
  Branches      456      453       -3     
==========================================
+ Hits          837      851      +14     
+ Misses       1949     1923      -26     
+ Partials      350      346       -4
Flag Coverage Δ
#integration 0% <ø> (ø) ⬆️
#unittests 27.9% <50%> (+0.6%) ⬆️
Impacted Files Coverage Δ
client/src/screens/groups/styles.js 0% <ø> (ø) ⬆️
client/src/components/Modal/styles.js 100% <ø> (ø) ⬆️
client/src/components/Modal/index.js 0% <0%> (ø) ⬆️
client/src/screens/groups/NewGroup.js 0% <0%> (ø) ⬆️
client/src/components/Modal/TagModal.js 90.9% <90.9%> (ø)
client/src/graphql/time-segment.mutation.js 100% <0%> (ø) ⬆️
client/src/screens/schedules/Detail.js 0% <0%> (ø) ⬆️
client/src/constants.js 100% <0%> (ø) ⬆️
client/src/screens/burger/components/Profile.js 100% <0%> (ø) ⬆️
client/src/selectors/schedules.js 63.88% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d6916e...d0ae2a9. Read the comment docs.

@OSPFNeighbour OSPFNeighbour force-pushed the tdykes.refactor_tags_to_modal branch from b5a2294 to 25c1920 Compare July 17, 2018 02:34
Copy link
Member

@sdunster sdunster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nits

<CheckBox
style={{ flex: 1, padding: 10 }}
rightText={option.name}
onClick={() => this.props.onSelect(option)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this was it's own component you wouldn't need the closure

round
showLoading={this.props.isLoading}
containerStyle={styles.searchbar}
onChangeText={text => this.props.onSearch(text)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 onChangeText={this.props.onSearch}

visible={this.state.tagsModal}
closeModal={this.handleTagBack}
onSearch={text => this.searchTagOnPress(text)}
onSelect={option => this.handleTagChange(option)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onSearch, onSelect and backModal can all drop the closure like this:

onSelect={this.handleTagChange}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well thats cool. TIL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants