ProductiveRage / Bridge.React

Bindings for Bridge.NET for React - write React applications in C#!
MIT License
74 stars 14 forks source link

Support for "data-" attributes #28

Closed Qboid closed 7 years ago

Qboid commented 7 years ago

Hi Productive Rage,

Could you add support for "data-" attributes please?

Many thanks!

badcommandorfilename commented 7 years ago

+1 with a more abstract goal for just adding general custom attributes to ReactElements.

I think you might be able to inherit from ReactDomElementAttributes to generate your own Attribute classes. It would be convenient to extend InputAttributes, but it's sealed.

ProductiveRage commented 7 years ago

@badcommandorfilename I'm not keen on the idea of InputAttributes being un-sealed for two reasons.

Firstly, the aim of the bindings is to add type safety to React and to avoid being able to set properties that React does not support for particular elements - if you could create a class that derived from InputAttributes then the C# type system would allow you to pass an instance of that class into the DOM.Input factory helper method but the new properties on the derived class would not be applicable to regular inputs.

Secondly, I'm not convinced that there's a good case to extend InputAttributes for your own types. If you had some sort of specialised or enhanced input element, would you really want it to support every input element option? For example, you would have to support every single InputType option - if you were creating some sort of enhanced text box element then the InputType value would not be applicable (you would have to ignore any value other "Text" or you would have to throw a runtime exception to say "only Text is supported" - which is moving away from compile time type safety). If you were creating some sort of labelled text input or text-input-with-validation-error component then it would make more sense to allow an input element as a child element - that way you don't need to try to pass all of the InputAttribute values directly into your custom component props.

I'm not saying that there are zero use cases where inheriting from the classes like InputAttributes would be appropriate but I do think that they would be edge cases and that it's a better compromise to leave them sealed.

ProductiveRage commented 7 years ago

I've added a way to add any "data-*" properties. The change has been pushed and should arrive in a Bridge.React pre-release NuGet package either tomorrow or early next week.

There is a new dynamic "Data" property that allows you to set any values that you want. Any property name within the Data object will be extracted into individual "data-*" properties when passed to React - eg.

new Attributes { ClassName = "toggler", Data = new { toggle_me = "yes-please" } }

will become

{ "className": "toggler", "data-toggle-me": "yes-please" }

Since C# does not allow hyphens in property names but these are quite common in data attribute names, any underscores in the Data reference property names will be replaced with hyphens (hopefully there aren't any / many "data-*" attribute names in use that want underscores!).

You can include as many or as few properties in the Data object as you want to. Ensure that you don't repeat the "Data" part of the names, though - note that in the above example, the Data property is called "toggle_me" and not "data_toggle_me".

kendallb commented 5 years ago

At what point does the data- attribute property get translated? I am trying to use it like this, but I am not sure the compiled code is going to do what I expect. Here is my render function:

        public override ReactElement Render()
        {
            var clonedProps = Helpers.CloneProps(props);
            clonedProps.AutoComplete = AutoComplete.Off;
            clonedProps.DecimalScale = 0;
            clonedProps.Data = new {
                lpignore = "true",
            };
            return new NumberFormat(clonedProps);
        }

The resulting output is this:

            render: function () {
                var clonedProps = Website.POS.UI.Helpers.CloneProps(System.Object, this.unwrappedProps);
                clonedProps.autoComplete = "off";
                clonedProps.decimalScale = 0;
                clonedProps.data = { lpignore: "true" };
                return React.createElement(NumberFormat, clonedProps);
            }

Since that is a direct call to React.createElement and the clonedProps are passed in directly, I don't think any translation from the data object to the individual data- properties will work? Or is the data object something that is natively supported in React? As referenced here, the standard way to pass data- and aria- attributes to the DOM in React is the same way you do in HTML:

https://reactjs.org/docs/dom-elements.html

So from what I can tell there is nothing in React that would understand how to process a data property with an object of expected attributes? Really the generated code really needs to somehow look like this:

            render: function () {
                var clonedProps = Website.POS.UI.Helpers.CloneProps(System.Object, this.unwrappedProps);
                clonedProps.autoComplete = "off";
                clonedProps.decimalScale = 0;
                clonedProps.data-lpignore = "true";
                return React.createElement(NumberFormat, clonedProps);
            }

Am I missing something here?

kendallb commented 5 years ago

Checking what gets generated for the DOM.Div() function, it products this output:

React.createElement('div', Bridge.React.fixAttr({ id: "message-stack-messages", className: "message-stack-messages", data: { my_data: "blah" } }))

from this binding:

    [Template("React.createElement('div', Bridge.React.fixAttr({0}), System.Linq.Enumerable.from({1}).ToArray())")]
    public static extern ReactElement Div(
      Attributes properties,
      IEnumerable<ReactElement> children);

So I think the solution is I need to add Bridge.React.fixAttr() to my own bindings to get the data- attributes to map? Looking at the code that would appear to be the case, so the NumberFormat constructor should look like this:

        [Template("React.createElement(NumberFormat, Bridge.React.fixAttr({props}))")]
        public NumberFormat(
            Props props)
        {
        }

and my Render function is this:

            render: function () {
                var clonedProps = Website.POS.UI.Helpers.CloneProps(System.Object, this.unwrappedProps);
                clonedProps.autoComplete = "off";
                clonedProps.decimalScale = 0;
                clonedProps.data = { lpignore: "true" };
                return React.createElement(NumberFormat, Bridge.React.fixAttr(clonedProps));
            }

Maybe the documentation on writing your own bindings can be updated to include this, so data- and aria- attributes would work?

Also looking at the code for fixAttr(), I think that function needs to also be updated to handle the aria- attributes as well?