-
Notifications
You must be signed in to change notification settings - Fork 83
Improve spec compliance for text-decoration #65
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
Conversation
src/transforms/textDecoration.js
Outdated
) { | ||
lines.push(tokenStream.lastValue) | ||
if (lines[0] === lines[1]) { | ||
throw new Error('Expected two different text decoration line styles') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding a test for this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. Not even sure why I added that. I should probably just let RN handle the invalid values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, to me it seems a bit weird that some CSS parsing errors are thrown by css-to-react-native
and others go all the way to RN. Of course in both cases the user will see the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think maybe the cut-off should be is if the parsing is ambiguous or incomplete we throw an error, otherwise let RN do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, that make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Oh yeah there is still the ongoing discussion about reordering values in the other PR, so if something is changed there it needs to be changed here too. |
No description provided.