wordpress-mobile / gutenberg-mobile

Mobile version of Gutenberg - native iOS and Android
GNU General Public License v2.0
243 stars 56 forks source link

Find common place for BREAKPOINTS for mobile #2129

Open jbinda opened 4 years ago

jbinda commented 4 years ago

Is your feature request related to a problem? Please describe.

Let me provide some background first: During work on Column block PR I need to base on container size to set the proper layout (vertical of horizontal). @lukewalczak introduce useResizeObserver hook (see here) that helps to measure size of Columns block container.

However I also need to compare container size with breakpoints to calculate how many Columns should be show in one row. Here is a guideline for how it should collapse depending on size. Just keep in mind that it is not the screen size but the size of the container because we can nest our Columns block inside another one and then the layout should be set depending on available space. Basing on screen size will cause that both of mentioned block will behave in the same way - thats not something that we would like to achieve

Please also check this Column PR comment to better understand why I have opened this issue.

No lets go to the problem:

To achieve above I need to introduce BREAKPOINTS object and after measure container size compare it to calculate number of Columns in row (like mentioned above)

The issue here is that we do not have common place when we can store this BREAKPOINTS thats why it is placed in Columns block edit.native.js file like below:

const BREAKPOINTS = {
    mobile: 480,
    large: 768,
};

Column block is the second block which use this object. It is also defined in Media-Text (gutenberg/packages/block-library/src/media-text/edit.native.js line 41). I believe here will be much more blocks in the future because basing on container size rather that the screen size is more reliable.

Describe the solution you'd like We would like to find the place where we can put this BREAKPOINTS object and import it whenever we would like to compare

Describe alternatives you've considered Lukes hook mentioned above in initial version allow to read the container size and compare to BREAKPOINTS defined in hook file. The goal was to return flag isMobile, isLarge without additional comprising. During discussion we based on useViewportMatch - but the main difference was to not read the device screen size but the block container size.

After long discussion the code changes and the decision was to provide a hook just to measure the size of View in which it will be applied. PLease see the (hook PR description) to better understand how to use it.

Summarising my point is that alternate solution to find a common place for BREAKPOINTS may be revise our initial idea and in e.g create new hook which will be modification of final version of Lukes hook which will return a boolean flag if the container size is in specific range defined in BREAKPOINTS object

Hope all above is clear enough to start discussion about how we would like to approach that.

At the end I would like to ask who else should be involved in this debt ?

cc @pinarol @lukewalczak @dratwas

chipsnyder commented 3 years ago

I'm going through and reviewing some of the open issues on our boards to make sure I understand the current state.

@lukewalczak With the work we've done on Columns since this issue was created, I'm wondering if this is still relevant or can it be safely closed?

lukewalczak commented 3 years ago

Hello @chipsnyder, thanks for kicking back that issue. I think it's still worth to investigate it and to introduce the mobile equivalent of useViewportMatch hook. During the work over full width columns I crafted small utility function called isWider to compare the passed width with the breakpoint, which can be replaced in favour of that hook. However, I would consider that as nice to have rather than must have.