[RNMobile] remove override support inserter for columns block #21016
Conversation
jbinda
added
Mobile App - i.e. Android or iOS
jbinda
requested review from
ajitbohra,
ellatrix,
jorgefilipecosta,
nerrad,
ntwb,
Soean,
talldan and
youknowriad
as code owners
jbinda
removed request for
youknowriad,
talldan,
Soean,
ntwb,
ajitbohra,
nerrad,
ellatrix and
jorgefilipecosta
|
Size Change: 0 B Total Size: 822 kB ℹ️ View Unchanged
|
jbinda
force-pushed
the
rnmobile/column-block-with-support-inserter
branch
from
82c4667 to
2f0496e
Compare
jbinda
requested review from
chrisvanpatten,
epiqueras,
gziolo and
mkaz
as code owners
jbinda
requested review from
noahtallen,
noisysocks and
TimothyBJacobs
as code owners
jbinda
removed request for
chrisvanpatten,
epiqueras,
gziolo,
mkaz,
noahtallen,
noisysocks and
TimothyBJacobs
jbinda
added
the
[Status] Blocked
… into rnmobile/column-block-with-support-inserter
jbinda
force-pushed
the
rnmobile/column-block-with-support-inserter
branch
from
42ce8d8 to
7b225d4
Compare
jbinda
changed the title
| @@ -21,7 +21,6 @@ export const settings = { | |||
| icon, | |||
| description: __( 'A single column within a columns block.' ), | |||
| supports: { | |||
| inserter: false, | |||
There was a problem hiding this comment.
I have a similar PR, I believe we need a notice in the UI somewhere when we reach the limit of max 6 columns. It becomes more of a guideline than a requirement.
There was a problem hiding this comment.
I merged your branch to this :)
On mobile side we temporary override support: inserter to true in index.native.js and will remove it after your PR gets merged.
Also Columns works slight different on mobile because we are wrapping them depending on screen size (this wrap happens more quicker than on the web so in the result we have matrix of Column). It do not visualise the layout in 100% but in the other hand we don't need to worry about the max limit according to breaking layout on mobile. However it would be nice to have some UI notice to be aware of having different layout on web.
I will post you a GIF tomorrow to show what I mean
There was a problem hiding this comment.
Below GIFs should visualise what I mean.
Vertical position
Horizontal position
Few side notes
Currently our guidelines according to looks like that: see this comment. However we also have hardcoded limitation on block container max-width set to 580px. The conclusion is that at this moment we should never have more that 2 Columns in the row on mobile. The limitation notice in Columns on mobile will have more sense if we can go above that width limit ( also above 768px). Then the Columns logic will try to render all Columns in one row. Still we can adjust to add some min-width limit from which we start to wrap in the same manner again. Maybe someday in the future we extend this max-width - don't know exact plan on this but on iPad it feels quite awkward because of lot of empty space on sides - specially in Horizontal layout. I know it's UX to increase readability but not sure if it wasn't also some workaround because of implementation. Please see below screen to see how it looks on iPad.
Apart of above if we have UI notice on web we also should have it on mobile to notice user that layout may be different on web. I'am fully agree with that. @iamthomasbishop do you have any thoughts how this notice should looks like ?
…ck-with-support-inserter
|
because Allow support inserter in Columns block PR is merged I have sync the PR and we can remove @pinarol do you think we can merge this or wait too see if mentioned PR wont be reverted according to latest comment |
jbinda
removed
the
[Status] Blocked
Description
🚧 NOTE: Merge only after Allow support inserter in Columns block PR will be merged. 🚧
This PR removes
support inserteroverrides made inindex.natie.jsinside Column Block.Please refer to:
Related gutenberg-mobile PR
Allow support inserter in Columns block PR
Column Block PR
design note
How has this been tested?
Checking all Column Block PR testing requirements (see Column PR description)
List of changes:
index.native.jsfrom column with overridessupport: insertersettingTypes of changes
Refactor - Change overrides of
support: inserteraftersupport inserter: falsefromindex.jsof column block prop will be delete.Checklist: