New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Alloy Editor to set one dimension when resizing an image #601

Closed
blzaugg opened this Issue Oct 7, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@blzaugg
Member

blzaugg commented Oct 7, 2016

From: https://issues.liferay.com/browse/LPS-68536

User Story:

As a developer, integrating Alloy Editor, I'd like a configuration option, to allow me to set image dimensions with width|height|both.

This would allow me better control, in how image scaling behaves in the resulting output. Without resorting to !important overrides.

Background:

See discussion: https://in.liferay.com/web/support/forums/-/message_boards/message/25343702

When resizing an image's height or width or both, height and width dimensions are set as inline styles on the image element.

When the resulting output image is displayed in a container smaller than the set static width, the image breaks out of the container.

This is typically fixed by setting max-width: 100% on the image, forcing it to resize.

An issue arises, that because height is set, the original ratio is not kept, causing the image to squish.

To further fix this, height must to set to auto, to allow the original ratio to remain intact. But because height as be set as an inline style, !important must be used to override the inline height.

@blzaugg

This comment has been minimized.

Member

blzaugg commented Oct 7, 2016

Possible solution, minus config option setup:

I'm pretty sure the code, that would need to change, would be:

https://github.com/liferay/alloy-editor/blob/master/src/plugins/dragresize.js#L428-L431

    function resizeElement(el, width, height) {
        el.style.width = String(width) + 'px';
        el.style.height = String(height) + 'px';
    }

I may be simplifying this, but I believe all it would take is to check for some config value: 

    function resizeElement(el, width, height) {
        if (config.resize === 'both') {
            el.style.width = String(width) + 'px';
            el.style.height = String(height) + 'px';
        }
        else if (config.resize === 'width') {
            el.style.width = String(width) + 'px';
        }
        else if (config.resize === 'height') {
            el.style.height = String(height) + 'px';
        }
    }
@ipeychev

This comment has been minimized.

Contributor

ipeychev commented Oct 8, 2016

Thanks for opening this issue. @jbalsas, could you please take a look?

@jbalsas

This comment has been minimized.

Member

jbalsas commented Oct 10, 2016

Yes, we'll take a look at this.

Right now, however, it would have lower priority than creating some small, medium and full-width buttons to simplify setting sizes to images.

I'm also concerned about how different handles should behave (maybe some of them shouldn't even appear) with the different settings for this.

antoniopol06 added a commit to antoniopol06/alloy-editor that referenced this issue Nov 2, 2016

antoniopol06 added a commit to antoniopol06/alloy-editor that referenced this issue Nov 2, 2016

antoniopol06 added a commit to antoniopol06/alloy-editor that referenced this issue Nov 2, 2016

@ipeychev ipeychev closed this in 46e6bf8 Nov 2, 2016

ipeychev added a commit that referenced this issue Nov 2, 2016

Fix describe and should statements in tests
Replace test method to check for IE using CKEDITOR.env.ie

Fixes #601

ipeychev added a commit that referenced this issue Nov 2, 2016

ipeychev added a commit that referenced this issue Nov 2, 2016

@ipeychev ipeychev added this to the 1.2.5 milestone Nov 2, 2016

azotova added a commit to azotova/alloy-editor that referenced this issue Jan 7, 2017

azotova added a commit to azotova/alloy-editor that referenced this issue Jan 7, 2017

azotova added a commit to azotova/alloy-editor that referenced this issue Jan 7, 2017

Fix describe and should statements in tests
Replace test method to check for IE using CKEDITOR.env.ie

Fixes liferay#601

azotova added a commit to azotova/alloy-editor that referenced this issue Jan 7, 2017

azotova added a commit to azotova/alloy-editor that referenced this issue Jan 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment