Avoiding placeholder collapse caused by flex siblings #439
Conversation
alexreardon
changed the title
| // Maps to the CSS box model | ||
| // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Box_Model/Introduction_to_the_CSS_box_model | ||
| /* | ||
| # The CSS box model |
There was a problem hiding this comment.
This is really the core change. Being more aware of the box model.
There was a problem hiding this comment.
Previously we where incorrectly labelling paddingBox as borderBox. Additionally, the contentBox for a Droppable was not correctly excluding borders. It was only subtracting padding which is not enough to get to the content box.
CSS!
| // Maps to the CSS box model | ||
| // https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Box_Model/Introduction_to_the_CSS_box_model | ||
| /* | ||
| # The CSS box model |
There was a problem hiding this comment.
Previously we where incorrectly labelling paddingBox as borderBox. Additionally, the contentBox for a Droppable was not correctly excluding borders. It was only subtracting padding which is not enough to get to the content box.
CSS!
src/view/placeholder/placeholder.jsx
Outdated
| boxSizing: 'border-box', | ||
| display, | ||
| // Avoiding the collapsing or growing of this element when pushed by flex child siblings. |
There was a problem hiding this comment.
Here is the placeholder fix
|
The box model changes fixes a bug where there incorrect movement into an empty droppable that has a border Before
After
|
| // We have already taken a snapshot the current dimensions we do not want this element | ||
| // to recalculate its dimensions | ||
| // It is okay for these properties to be applied on elements that are not flex children | ||
| flexShrink: '0', |
There was a problem hiding this comment.
This is the fix to the first issue: avoiding dynamic changes to the placeholder if it is a flex sibling
| @@ -272,8 +270,18 @@ export default class DroppableDimensionPublisher extends Component<Props> { | |||
| bottom: parseInt(style.paddingBottom, 10), | |||
| left: parseInt(style.paddingLeft, 10), | |||
| }; | |||
| const border: Spacing = { | |||
There was a problem hiding this comment.
droppable contentBox now correctly accounting for borders
|
/cc @jaredcrowe as we have talked about this problem space before |
Change 1: placeholder
When a placeholder is placed, it can be collapsed by flex siblings as it might have a smaller 'width' property. By avoiding flexshrink and flexgrow we avoid visual jank.
Use case:
By opting out of flex-grow and flex-shrink on the placeholder we avoid the placeholder being pushed or pulled by flex siblings
Change 2: internal box model cleanup
I have gone deep and improved the internal box model to be more accurate