fix: improve drag handler visibility on IE (#1255) #1256
Conversation
|
Just started reviewing :) |
wincent
changed the title
src/plugins/dragresize_ie11.js
Outdated
| dragHandlerStyle.setAttribute('backgroundColor', 'rgba(255, 255, 255, 1'); | ||
| dragHandlerStyle.setAttribute('opacity', '1'); | ||
| dragHandlerStyle.setAttribute('margin-top', '-5px'); | ||
| dragHandlerStyle.setAttribute('margin-left', '-20px'); |
There was a problem hiding this comment.
Wondering if this would belong better inside setupResizer()? I don't know if you consider it to be related to that, but it looks that way to me.
There was a problem hiding this comment.
I think it's technically separate so maybe I should move it out of the if block since I'm not sure that the resizing and the drag and drop handle are explicitly linked functionality
|
/cc @antonio-ortega |
gregory-bretall
force-pushed
the
drag-handler-visibility-fix
branch
from
b3c739c to
62cf005
Compare
|
I made some changes to the branch including moving it outside the if check being used for setupresizer (since I think the drag and drop functionality and resize functionality are supposed to be mutually exclusive) and changed it from var to const |
…to make it stand out more and move the handle so it is more visibly separate from the image
gregory-bretall
force-pushed
the
drag-handler-visibility-fix
branch
from
62cf005 to
cf60c03
Compare
|
Note: I made another change removing positioning changes as it can potentially conflict with a possible fix to https://issues.liferay.com/browse/LPS-89596 which was originally added to liferay-ckeditor but seems to have not been transferred over to CKEditor 4.11.3. I will look into fixing that again later, but for now not touching the drag handle positioning seems like the best plan. |
Good catch. When we did the big CKEditor update we changed the build process quite dramatically, so this will be the first change that we need to integrate using the new workflow. I don't know what the original fix was (because the LPS just updates the version number), but I'm wondering: was it the kind of fix that could potentially be upstreamed into CKEditor itself? In any case, in the meantime this PR looks good and low-risk so let's go ahead and merge it. Thanks for providing the screenshots to make clear what the before/after looks like. |
wincent
changed the title
|
See tag 4.6.0.5 for LPS-89596 changes. |
|
This is the specific commit: liferay/liferay-ckeditor@4d8ce3e |
|
Sorry, I was including the range from the previous tag, so all relevant commits were included. |
Make the drag handler opaque, change the color to white to make it stand out more and move the handle so it is more visibly separate from the image
Fixes #1255
I decided to add the styles inline as CKEditor by default already adds the styles in-line and this seemed like a cleaner implementation within the CKEditor framework than trying to implement it using real CSS.