search

Tradeshift / tradeshift-ui

Tradeshift UI is a framework-agnostic JavaScript library to help Tradeshift App developers to create cohesive user experiences and to provide reusable UI components.
https://ui.tradeshift.com
Other
33 stars 44 forks source link

[Forms] `data-ts.icon` is not supported in JSX #203

Open sampi opened 7 years ago

sampi commented 7 years ago

Tradeshift UI version affected

v7.0.0

Expected Behavior

I can use data-ts.icon attributes on my <input /> fields

Actual Behavior

I get a Syntax error: Unexpected token message

Steps to reproduce

Write <input data-ts.icon="ts-icon-menuswitch" /> anywhere on a JSX page and try to compile it.

sampi commented 7 years ago

This might be a fix: https://gist.github.com/chinchang/a66fe3aedaf98b2433eb

RealDeanZhao commented 7 years ago

workaroud: 

const topBarAttr = {
    'data-ts': 'TopBar',
    'data-ts.title': 'asdfasdft'
}
render(
 <header {...topBarAttr}></header>
)
wiredearp commented 7 years ago

I remember when we introduced this, JSX did not support custom attributes like ts-aside (it does now) but it did support the the attribute in question, data-ts.icon (it doesn't now). So this appears to be a moving target. Not sure what to do about it :cry: it would hurt to change this once again.

DocGroth commented 7 years ago

Reports are coming in that it is not working in Angular 2 as well.

jypma commented 6 years ago

I'm having similar issues with the data-ts.title for asides. My framework seems to be preferring the data-ts-title convention, and I see some data-ts-spirit in the DOM as well. I also see nobody else using dots in data attributes, and it seems to be discouraged.

Perhaps start moving to only using dashes? or at least as an alternative.

wiredearp commented 6 years ago

@jypma We moved from dashes to dots precisely because dashes were at that time not supported in React, but that of course changed the week after and now React has a problem with dots; although it is still possible to get the attributes declared if you have the patience. It's a huge operation to change the syntax (again), but we agree that it should be done 😞