aimeos / ai-cms-grapesjs

GrapesJS CMS integration into Aimeos
GNU Lesser General Public License v2.1
22 stars 18 forks source link

Columns in Grape Editor are hard to manipulate #7

Closed Dahercode closed 3 years ago

Dahercode commented 3 years ago

Hello Aimeos, I added grapejs to my aimeos project (Laravel), and i figured that the editor in the content section (JQADM) has a small issue : After inserting the columns, it's really hard to insert text or any content inside.

Is it a general issue ?

Thank you

aimeos commented 3 years ago

That depends on what you think the problem is. If the column is too small, you could add more padding or add a minimum height for columns: https://github.com/aimeos/ai-cms-grapesjs/blob/master/admin/jqadm/themes/custom.js#L418

Dahercode commented 3 years ago

That depends on what you think the problem is. If the column is too small, you could add more padding or add a minimum height for columns: https://github.com/aimeos/ai-cms-grapesjs/blob/master/admin/jqadm/themes/custom.js#L418

Thank you, it solved the issue. I think it's better to put a bigger padding by default, the 10 px value is nearly impossible to work with (I may be wrong^^)

aimeos commented 3 years ago

A padding has the problem that is may interfere too much with the design of the frontend. Is this change enough for you?

.gjs-dashed .row, .gjs-dashed .col, .gjs-dashed [class^="col-"] {
    min-height: 3rem !important;
}
Dahercode commented 3 years ago

Understood, but it's not enough, i'm obliged to add it in .row .row { display: flex; padding: 10px 0; width: auto; min-height: 3rem; } The div class value is only set to "row" when the columns are not hovered (row gjs-hovered) or not selected (row gjs-selected)

aimeos commented 3 years ago

If you need more space between the row and the columns, we should use the padding instead. Is it enough to increase the row padding from 10px to 20px?

Dahercode commented 3 years ago

Indeed, i think it's enough, but, it still has a weird behaviour compared to adding "min-height". I recorded quikcly the impact (Orginal code / Increasing padding / Adding a min-height)

aimeos commented 3 years ago

Your screenshots always shows the same image so we can't see what you mean

Dahercode commented 3 years ago

It's a .gif (animated screenshots), you just have to wait some seconds to charge :D

aimeos commented 3 years ago

OK, can you create a pull request with the necessary changes?

Dahercode commented 3 years ago

Sure, can you first choose beetween the last 2 behaviours ? ( With padding or min-height )

aimeos commented 3 years ago

Think, the min-height way would be preferrable

Dahercode commented 3 years ago

Done : https://github.com/aimeos/ai-cms-grapesjs/pull/8