GrapesJS / grapesjs

Free and Open source Web Builder Framework. Next generation tool for building templates without coding
https://grapesjs.com
BSD 3-Clause "New" or "Revised" License
22.38k stars 4.06k forks source link

Color picker displays wrong position #596

Closed TrungPham383 closed 6 years ago

TrungPham383 commented 6 years ago

add

artf commented 6 years ago

Are you able to provide a live example of the issue? (jsfiddle, codepen, etc..)

TrungPham383 commented 6 years ago

https://drive.google.com/open?id=1h62HYZcg8M-YIwP-jQUxiGO51QDt3_ml Here is video for this bug.

AJOVTIC commented 6 years ago

I have the same problem

BaptisteLeBescond commented 6 years ago

Same here ! :)

TrungPham383 commented 6 years ago

If use grapesjs old version, it will work normal. But color or backround color not working. :D

BaptisteLeBescond commented 6 years ago

I use grapesjs - 0.12.52 (last version I think). I also have the plugin grapesjs-mjml but the problem remains if I remove it. Not related I guess.

EDIT: Ok so it works in Full screen. Problem is my editor is embedded in my app. Its size is not window size.

TrungPham383 commented 6 years ago

Vào Th 2, 11 thg 12, 2017 lúc 21:29 BaptisteLeBescond < notifications@github.com> đã viết:

I use grapesjs - 0.12.52 (last version I think). I also have the plugin grapesjs-mjml but the problem remains if I remove it. Not related I guess.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/artf/grapesjs/issues/596#issuecomment-350739608, or mute the thread https://github.com/notifications/unsubscribe-auth/AgjCNlMCDhlLDOVhcF4B5a-7ojRjzYwrks5s_TxBgaJpZM4QwcQ3 .

Yes, version 0.9.7 work good but color not apply on view

artf commented 6 years ago

Ok, I guess there is something wrong with the calculation of the position when the editor doesn't fit the window size

Santieherrero commented 6 years ago

Hi, I think that the issue it's here, at this two lines #1086 at 'ColorPicker.js':

var doc = picker[0].ownerDocument;
var docElem = doc.documentElement;
...
var cW = docElem.clientWidth;
var cH = docElem.clientHeight;
...

It's taking the main document when it's getting the offset, should not it be just the container of grapesjs init?

swarnat commented 6 years ago

I had exactly the same problem and thanks to your pointer to ColorPicker.js I was able to fix.

Like artf say, the calculation of position is wrong, when editor is shown with a popup, like bootstrap modal. So I replaced the correction lines here like this:

offset.left -= Math.min(
      offset.left,
      offset.left + dpWidth > viewWidth && viewWidth > dpWidth
        ? Math.abs(offset.left + dpWidth - viewWidth)
        : 0
);

offset.top -= Math.min(
      offset.top,
      offset.top + dpHeight > viewHeight && viewHeight > dpHeight
        ? Math.abs(dpHeight + inputHeight - extraY)
        : extraY
);

to a simple

    offset.left += input.outerWidth();
    offset.left -= input.closest(".gjs-editor").offset().left;
    offset.left -= dpWidth;

    offset.top += inputHeight;
    offset.top -= input.closest(".gjs-editor").offset().top;

This works perfect for me, because I took input offset and subtract the offset of popup and datepicker. to more.

I only don't create a Merge request, because I think it isn't the best way to handle this. Also the top position must better recognize browser borders. I will try to optimize the change and create some full page test cases over weekend, because I love this editor. There is also probably a default way to get container element, which I oversee during code research.

artf commented 6 years ago

Thanks @swarnat I really appreciate your efforts, a PR on this is highly welcome

frasza commented 6 years ago

Hey, @artf , any word on this? We have quite difficult time dealing with inaccessible color picker.

fguslinski commented 6 years ago

Hi there, I solved this using the following configuration:

grapesjs.init({ colorPicker: { appendTo: 'parent', offset: { top: 26, left: -166, }, }, })

Maybe you need to adjust the top and left positions for you, because I changed some sizes. But works perfect here.

artf commented 6 years ago

@TrungPham383 does the @fguslinski solution work for you? I wasn't even able to reproduce it (I suppose it happens when the editor is in some particular external layout, would be great if someone could indicate a reproducible case)

no-response[bot] commented 6 years ago

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

cmoutafidis commented 6 years ago

@artf In the application I am working on the user can scroll vertically. grapesjs is loaded inside an iframe. The color picker is displayed with different left values before and after vertical scroll. So if I scroll to the left, the color picker will have a higher left value. But if I scroll to the right, the color picker will have a smaller left value. That is not really correct because the style property element is not actually moving, only the preview window does. As @fguslinski mentioned above, if I define a standard value for the left property, e.g. 1000px, the issue is "resolved". The problem is that not everyone has the same monitor, so this is not a resolution. For example, if I open the devTools to the right of the browser, color picker's position is ruined again.

cmoutafidis commented 6 years ago

@swarnat suggested a great solution, as the color picker's left value is not affected by vertical scroll.

artf commented 6 years ago

@cmoutafidis are you able to re-create the layout situation which shows the issue, please?

markCornejo commented 6 years ago

@fguslinski

grapesjs.init({ colorPicker: { appendTo: 'parent', offset: { top: 26, left: -166, }, }, })

This did not solve the problem, the variable 'colorPicker' does not work for me. I use "grapesjs": "0.12.17". Who has some other alternative?

arachnosoft commented 5 years ago

Just to add some follow-up and updated info on this issue. We also had it on the latest grapesjs (0.14.52)... but only on Firefox.

Our context is roughly the same as @cmoutafidis, @BaptisteLeBescond and @swarnat , grapesjs is embedded inside an iframe (which is a custom "modal" window built just as those from CKEditor or grapesjs).

The color picker was displayed like this:

grapesjs color picker - firefox

Here is the corresponding F12 console from Firefox:

grapesjs color picker - firefox - f12

I used the configuration tip provided by @fguslinski , and it works on most cases:

colorPicker: { appendTo: 'parent', offset: { top: 20, left: -175 } },

Gives this:

grapesjs color picker - firefox after fix

image

It seems to work on several browsers and screen resolutions, but this fix may not be perfect, as it relies on somewhat random offset coordinates depending of our own appreciation.

@artf , just to give you a (very) raw idea of our HTML page structure, in case it'd help you reproducing the issue... but I doubt that you'll have enough information to do so.

image

Here is the computed CSS given by Firefox on the color palette DOM item:

image

image

image

Feel free to ask for more information, I'll try to provide what you need if you want to investigate furthermore on this issue (not really needed on my side thanks to @fguslinski 's fix which worked for me, but not for @tfalcom , apparently).

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.