Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@danwalmsley
Copy link

Hello all,

This PR addresses issues #108 and #517. I also added a few unit tests to cover this scenario.

:)

@danwalmsley
Copy link
Author

Looks like one of my unit tests failed. I will check why and update.

@grokys
Copy link
Contributor

grokys commented Sep 20, 2016

Yeah, it's failing here locally as well.

@danwalmsley
Copy link
Author

@grokys I corrected logic and fixed unit tests now :)

filterTextIsEnabled = this.WhenAny(x => x.IsLoading, x => x.Value)
.Select(x => !x && repositories.UnfilteredCount > 0)
filterTextIsEnabled = this.WhenAny(x => x.IsLoading,
(loading) => loading.Value || repositories.UnfilteredCount > 0 && !LoadingFailed)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny nit: we usually don't put brackets around a single lambda parameter.

Copy link
Author

Choose a reason for hiding this comment

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

Let me correct this before you merge :)

Copy link
Author

Choose a reason for hiding this comment

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

ok fixed this.

@grokys
Copy link
Contributor

grokys commented Sep 22, 2016

Thanks @danwalmsley LGTM! If you could merge the latest changes from master in, I can merge.

@danwalmsley
Copy link
Author

Should be ready if you guys are happy :)

@grokys grokys merged commit 0328c97 into github:master Sep 27, 2016
@grokys
Copy link
Contributor

grokys commented Sep 27, 2016

Merged 🎉 - thanks Dan!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants