Skip to content

Data fields should be added #89

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

Closed
immae opened this issue Apr 2, 2019 · 10 comments · Fixed by #102
Closed

Data fields should be added #89

immae opened this issue Apr 2, 2019 · 10 comments · Fixed by #102

Comments

@immae
Copy link

immae commented Apr 2, 2019

All html attributes support arbitrary attributes like "data-key", which are not currently in the SharedProps type. Would it be possible to add them, for instance as a data :: Object String which would translate to data-key = "values" attributes?

@tesujimath
Copy link
Contributor

Are you aware that you can work around this limitation, like this?

element (unsafeCreateDOMComponent "button")
      { className: "navbar-toggler"
      , type: "button"
      , "data-toggle": "collapse"
      , "data-target": "#navbarSupportedContent"
      , "aria-controls": "navbarSupportedContent"
      , "aria-expanded": "false"
      , "aria-label": "Toggle navigation"
      , children: [ R.span { className: "navbar-toggler-icon" } ]
      }

@immae
Copy link
Author

immae commented Apr 11, 2019

Ah, no I’m still a beginner in purescript, thanks for the hint.

Though it may be good to have a mechanism for that kind of keys to avoid being obliged to use a workaround

@tesujimath
Copy link
Contributor

@immae I don't disagree with that point ;-)

@maddie927
Copy link
Member

This might be possible, I'll give it a try

@i-am-the-slime
Copy link
Member

I'm trying to use react-testing-library and it works, but the workaround is pretty painful. If at least there was support for data-testid it would help me greatly :)

@i-am-the-slime
Copy link
Member

@spicydonuts Did you get around to this? Could you give an outline of your idea so that I or somebody else might try?

@maddie927
Copy link
Member

What do you think of this approach? I used _data :: Object String instead of data, as some elements already have a data attribute.

R.div { _data: fromHomogeneous { hmm: "something" } }
<div data-hmm="something"></div>

@immae
Copy link
Author

immae commented Aug 24, 2019

@spicydonuts : this looks good, thanks for the work!

@i-am-the-slime
Copy link
Member

Nice, nice, nice, it works perfectly!

How about taking a homogenous Record right away so it looks a bit prettier at use site? Also maybe "data-" as the name? I'm not sure about this one because of the quotes, but I like that it's the exact prefix.

@maddie927
Copy link
Member

The problem with taking records directly is the extra type variable for the fields. If you don't supply a record (all the fields are optional) the compiler can't infer what the type is supposed to be, which is an error. If we could tell the compiler it's fine to default to an empty record it would work..

Yeah, I thought about that. The quotes seemed kind of annoying, but it is nice that it reads like the end result. Also I already released it so it's probably not worth it to change it 😅

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 a pull request may close this issue.

4 participants