Skip to content
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

Try: Absolutely positioned frame view approach to simplify the logic for border/space calculations of InnerBlocks #1948

Closed
pinarol opened this issue Feb 24, 2020 · 2 comments · Fixed by #2050

Comments

@pinarol
Copy link
Contributor

pinarol commented Feb 24, 2020

I forked this RN starter project and tried a different approach about putting borders & spaces around different blocks. https://github.com/pinarol/react-native-starter

The example is in a super primitive state but I tried to write down my mindset in code comments here: https://github.com/pinarol/react-native-starter/blob/124a7e9d4d9060753760b996957c708823da8434/src/modules/home/HomeView.js#L60

test

The frame view is absolutely positioned behind the blue view, but it is not blue view’s parent, they are at the same level

So we don’t need to make calculations for blue view’s paddings/margins/borders for different states.

All we need to do is positioning the frame view relative to blue view.

So when ready, we can give this approach a chance

cc @jbinda @lukewalczak

@pinarol pinarol changed the title Absolutely positioned frameview approach to simplify the logic for border/space calculations of InnerBlocks Absolutely positioned frame view approach to simplify the logic for border/space calculations of InnerBlocks Feb 24, 2020
@pinarol pinarol changed the title Absolutely positioned frame view approach to simplify the logic for border/space calculations of InnerBlocks Try: Absolutely positioned frame view approach to simplify the logic for border/space calculations of InnerBlocks Feb 24, 2020
@dratwas
Copy link
Contributor

dratwas commented Mar 11, 2020

Branch with ongoing work - https://github.com/WordPress/gutenberg/tree/rnmobile/block-borders

I'm trying the approach with borders positioned absolutely. In the current implementation, they are a part of the block. So in simplification - if we show the solid border, the main container is a view with border and some padding, then we have a container with dashed border and some paddings, and finally, we have our block. Now I removed these paddings and added borders as a view that is positioned absolutely relative to the block. Thanks to that we do not have to check if the solid /dashed border is shown, then remove some paddings but it brings some new areas to discuss. Mostly the spacing between blocks. It is easy for not nested block but if we use i.e group we need to handle a bit more. I listed some thoughts below.

Not nested block

It is a straight forward when we have not nested block - just add a margin on every side.
oneblock

Block in a group

We need to remove the horizontal margin for the nested block and leave them only at the root level.

We can not add a margin to the block inside and to the parent because we will have 2 margins on the very top and on the very bottom. We need to remove the margin for every first block in inner blocks and in the last block, with one exception for the last block - we need to add a margin for the last block if the parent is selected or add that margin to the inner block appender.

On the image below - the group with only one block inside is selected. The Block inside has the only bottom margin because its parent is selected (margin should not be added if the parent is not selected).

groupone

Floating toolbar in a nested block

There is also an issue with a floating toolbar in the first nested block. After I removed the margin-top from the first nested block, the floating toolbar is rendered right before the block w/o any spacing.
groupone

So I need to add margin-bottom to the floating toolbar of the first block. But it's not enough because there is extra space on the top because of the parent margin and the block is jumping.
jumpingblock

I think that the easiest way to solve it is setting the margin-top of the floating toolbar to -16 in the first block if its parent is selected. With that margin, the content is not jumping but I'm still worried about the columns block because they are positioned horizontally.
cc: @jbinda
notjumping

I know that there is also an issue with media-text block since it has some margins/paddings.

In my opinion, it is worth refactoring the borders and discuss all the margins/padding because thanks to that we wouldn't need to calculate that much. In the current implementation margins/padding depends also on borders - something like, if the border is visible then the margin has to be smaller because the border has width = 1 etc. After the refactor the problem doesn't exist.

@pinarol
Copy link
Contributor Author

pinarol commented Mar 11, 2020

Great progress! From what I have seen in the code it is worth refactoring too.

I had added some ideas here about what we can do in both horizontal & vertical stacking cases if that helps. Thanks for all the effort! Very excited to get this going.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants