Uh oh!
There was an error while loading. Please reload this page.
feat: Add Fabric implementation of inset logical properties - #35692
feat: Add Fabric implementation of inset logical properties#35692gabrieldonadel wants to merge 2 commits into
Conversation
analysis-bot
commented
Dec 21, 2022
Base commit: 9f78517 |
Base commit: 9f78517 |
pull-bot
commented
Dec 21, 2022
PR build artifact for d833603 is ready. |
| } | ||
| if (!props.insetBlockEnd.isUndefined()) { | ||
| result.position()[YGEdgeBottom] = props.insetBlockEnd; | ||
| } |
There was a problem hiding this comment.
We should give top/bottom precedence over BlockStart/BlockEnd, to match the resolution Yoga will do when given Start/End vs Left/Right. I.e. cardinal direction wins over flow relative direction. The group below this one can be matched for props which lose precedence to ones already set.
I think all the other values should be fine as is. E.g. “insetInlineStart” is the standard conformant name, so it should clobber RN’s “start”. Horizontal/Vertical and All don’t have existing props, so precedence doesn’t matter for them.
There was a problem hiding this comment.
got it @NickGerleman, I've just pushed a commit fixing this
NickGerleman
left a comment
There was a problem hiding this comment.
Thanks for the contribution. As a heads up, I’m mostly off the grid until January. Your last change should also be ready to merge internally, but I’m wanting to follow up with a screenshot test that we can’t do in OSS right now, that’s on my queue.
facebook-github-bot
commented
Dec 21, 2022
@ryancat has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
pull-bot
commented
Dec 21, 2022
PR build artifact for b4cd397 is ready. |
facebook-github-bot
commented
Dec 22, 2022
@gabrieldonadel would you be able to make a PR to https://github.com/facebook/react-native-deprecated-modules/tree/main/deprecated-react-native-prop-types with these changes as well? Same for the last PR once that is merged. There's a non-ideal situation where we deprecated RN's provided prop-types and moved them to a different repo, but we recently un-deprecated it after feedback we removed it too quickly. It is bundled (and enforced) as part of RN again, but we haven't yet brought the code back to the repo. In the meantime, prop changes end up touching three different repos (react-native, react-native-deprecated-modules, react-native-website). Though the issue will be better once prop-types are removed in a couple releases, or if we can copy the package back to the RN monorepo before then (FYI @hoxyq). |
gabrieldonadel
commented
Dec 22, 2022
Sure thing @NickGerleman, here is the PR -> facebook/react-native-deprecated-modules#12 |
…eact#35692)" This reverts commit 9669c10.
Summary
This PR implements
insetlogical properties as requested on #34425. This implementation includes the addition of the following style propertiesinset, equivalent totop,bottom,rightandleft.insetBlock, equivalent totopandbottom.insetBlockEnd, equivalent tobottom.insetBlockStart, equivalent totop.insetInline, equivalent torightandleft.insetInlineEnd, equivalent torightorleft.insetInlineStart, equivalent torightorleft.Changelog
[GENERAL] [ADDED] - Add Fabric implementation of inset logical properties
Test Plan
ViewpageInsetssection