Add TableCell writingMode #4437#4438
Conversation
ibastawisi
requested review from
acywatson,
fantactuka,
thegreatercurve,
tylerjbainbridge and
zurfyx
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
CLA Signed
zurfyx
left a comment
zurfyx
left a comment
There was a problem hiding this comment.
Thank you! Let me start by saying that this is a nice feature so thank you for working on this
| ); | ||
| cellNode.__rowSpan = node.__rowSpan; | ||
| cellNode.__backgroundColor = node.__backgroundColor; | ||
| cellNode.__writingMode = node.__writingMode; |
There was a problem hiding this comment.
In retrospective, having a __backgroundColor property isn't great. That was my bad, for some reason I thought that cells would never need any more styling properties and that they could be done on child nodes.
Can we copy TextNode getStyle()? Maybe the caching part isn't too critical for now but the property is important.
The reason why we have to do this before we merge your PR is backward compatibility. Once we commit to supporting this prop it will be hard to roll it back.
The second (minor) is because we probably don't want to commit on it officially until we address the overflow-x issue somehow:
There was a problem hiding this comment.
I agree that all CSS styles should go in one node property.
regarding the overflow issue we can hide the table cell overflow and also inherit the tr height to allow cell resizing:
.PlaygroundEditorTheme__tableCell {
height: inherit;
overflow: hidden;
}I might give it a try soon.
|
The previous commit breaks Cell background color feature disabled test which is expected. |
|
oh the serialized style property is breaking some unit tests as well, my bad! |
3 participants
No description provided.