Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issue with NSControl.rx.value multiple observers #1399

Closed

Conversation

fpillet
Copy link
Contributor

@fpillet fpillet commented Sep 5, 2017

When multiple observers want to look at NSControl.rx.value, only one would get updates because
the returned observable wasn't shared(). This is examplified by the new test added to NSButton

This also solves the issue of observers with varying types

Relates to issue #1398

When multiple observers want to look at NSControl.rx.value, only one would get updates because
the returned observable wasn't shared(). This is examplified by the new test added to NSButton

This also solves the issue of observers with varying types
@RxPullRequestBot
Copy link

1 Warning
⚠️ Please target PRs to develop branch

Generated by 🚫 Danger

@kzaher kzaher changed the base branch from swift4.0 to develop September 10, 2017 15:21
@kzaher kzaher changed the base branch from develop to swift4.0 September 10, 2017 15:22
Copy link
Member

@kzaher kzaher left a comment

Choose a reason for hiding this comment

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

I've cherry-picked your commit and created another one because we need to also apply it to develop branch.
Github probably won't detect the rebase and merge properly.

.takeUntil((control as! NSObject).rx.deallocated)
.share(replay: 1, scope: .forever)
}
.map { [unowned control] _ in getter(control) }
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be weak.

.takeUntil((control as! NSObject).rx.deallocated)
}
.takeUntil((control as! NSObject).rx.deallocated)
.share(replay: 1, scope: .forever)
Copy link
Member

Choose a reason for hiding this comment

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

I think that this should be .whileConnected.

return observer
}
.takeUntil((control as! NSObject).rx.deallocated)
}
.takeUntil((control as! NSObject).rx.deallocated)
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately for some reason still doesn't work with as control.rx.deallocated, have no idea why.


XCTAssertEqual(button.state, NSControl.StateValue.on)
XCTAssertEqual(value1, NSControl.StateValue.on)
XCTAssertEqual(value2, NSControl.StateValue.on)
Copy link
Member

Choose a reason for hiding this comment

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

We should also test replay.

@kzaher kzaher closed this Sep 10, 2017
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.

None yet

3 participants