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

Add attributes support to New #1

Closed
dphfox opened this issue Jun 23, 2021 · 5 comments · Fixed by #199
Closed

Add attributes support to New #1

dphfox opened this issue Jun 23, 2021 · 5 comments · Fixed by #199
Assignees
Labels
kind: enhancement New feature or request status: needs design Good idea but needs design work

Comments

@dphfox
Copy link
Owner

dphfox commented Jun 23, 2021

Currently it's not possible to set attributes on instances being created with the New function. Additionally, it's not possible to listen for changes on attributes.

While attributes aren't particularly useful within Fusion, having support for them would make it easier to integrate Fusion with legacy UI codebases which depend on them.

@dphfox dphfox self-assigned this Jun 23, 2021
@dphfox dphfox added the kind: enhancement New feature or request label Jun 23, 2021
@dphfox
Copy link
Owner Author

dphfox commented Jun 23, 2021

Possible API (implemented as part of the New function):

New "Instance" {
    [Attribute "Foo"] = "bar", -- if a state or computed object, binds to the attribute
    [AttributeChange "Foo"] = function(newValue)
        print("The attribute was changed to:", newValue)
    end
}

@dphfox
Copy link
Owner Author

dphfox commented Jun 23, 2021

An alternate possible API from Twitter (adapted from a suggestion by @/bradsharppp):

New "Instance" {
    [Attributes] = {
        Foo = "bar",
        [OnChange "Foo"] = function(newValue)
            print("The attribute was changed to:", newValue)
        end
    }
}

Alternatively, we could keep AttributeChange separate from the Attributes table?

@dphfox dphfox added the status: evaluating Currently gauging feedback label Jun 23, 2021
@dphfox dphfox added this to To do in Fusion v0.2 Jun 23, 2021
@blake-mealey
Copy link
Contributor

I personally prefer the first option as it is very similar to the API for normal properties.

@dphfox
Copy link
Owner Author

dphfox commented Jun 24, 2021

I personally prefer the first option as it is very similar to the API for normal properties.

Agreed - I don't personally see many use cases where a lot of attributes are being used at once within the context of Fusion, so I think it'd overall be cleaner not to use a nested table.

@Gargafield
Copy link
Contributor

I personally prefer the first option as it is very similar to the API for normal properties.

Agreed - I don't personally see many use cases where a lot of attributes are being used at once within the context of Fusion, so I think it'd overall be cleaner not to use a nested table.

Same here! I think fusion should eliminate the times where you normally in Roact get huge files with lots of scopes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement New feature or request status: needs design Good idea but needs design work
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants