Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Expose access to GetPropertyChangedSignal #37

Closed

Conversation

AmaranthineCodices
Copy link
Contributor

Allows access to property change events using GetPropertyChangedSignal via Roact.Change.[EventName]. This fixes #28.

This will behave like other events; one connection per object, etc.

Note that the vast majority of this is not unit tested. Lemur doesn't implement GetPropertyChangedSignal x.x

Example usage:

local Roact = require(game.ReplicatedStorage.Roact)

local test = Roact.createElement("ScreenGui", {}, {
	Roact.createElement("TextBox", {
		Position = UDim2.new(0.5, 0, 0.5, 0),
		Size = UDim2.new(0, 200, 0, 40),
		[Roact.Change.Text] = function(rbx)
			print("text changed to", rbx.Text)
		end,
		Font = "SourceSans",
		TextSize = 18,
	})
})

Roact.reify(test, game.Players.LocalPlayer:WaitForChild("PlayerGui"))

@coveralls
Copy link

Coverage Status

Coverage decreased (-3.03%) to 81.765% when pulling 818dd4a on AmaranthineCodices:property-changes into e554ad9 on Roblox:master.

@LPGhatguy
Copy link
Contributor

LPGhatguy commented Mar 5, 2018

Ergh. Maybe we should implement support for GetPropertyChanged in Lemur before this lands. I definitely want to improve test coverage as much as possible, which will probably involve rewriting the reconciler tests.

We didn't have test coverage metrics for Lua code when the majority of the Roact tests were written! D:

@AmaranthineCodices
Copy link
Contributor Author

I would love to get support for GetPropertyChangedSignal in Lemur. I'm not familiar enough with Lemur's codebase and how it does events to implement it though :(

@LPGhatguy LPGhatguy added the status: blocked externally Cannot be resolved until an issue in another project is closed. label Mar 9, 2018
@LPGhatguy
Copy link
Contributor

Stuff is in Lemur!

@LPGhatguy LPGhatguy removed the status: blocked externally Cannot be resolved until an issue in another project is closed. label Mar 17, 2018
@AmaranthineCodices
Copy link
Contributor Author

Cool! I'll try to get to this sometime this weekend.

@AmaranthineCodices
Copy link
Contributor Author

AmaranthineCodices commented Mar 17, 2018

This was not intended to drag in 27 commits >.<

Going to rewrite the branch history. Again.

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.

Expose GetPropertyChangedSignal declaratively
3 participants