Maintain image aspect ratio while resizing

closes RCX-2213
flag=block_editor

test plan:
  - create a page with an image block
  > expect the default Constraint to be Match Aspect Ratio
  - upload an image
  > expect the image to be in its natural aspect ratio
  - resize using the mouse
  > expect the image to remain in its aspect ratio
  - resize using the keyboard (opt-arror or shift-opt-arrow)
  > expect the image to remain in its aspect ratio
  - resize using the popup from the toolbar
  > expect the image to remain in its aspect ratio
  - change the constraint to something else
  - resize the image
  > expect it to resize to the requested size, ignoring the aspect ratio
  - try the above with a tall image and with a wide image

Change-Id: I2ea1cc0cd4f6958d0b091a27d8fede05a7895caf
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/355308
Reviewed-by: Jacob DeWar <jacob.dewar@instructure.com>
QA-Review: Jacob DeWar <jacob.dewar@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
This commit is contained in:
Ed Schiebel 2024-08-16 16:31:06 -06:00
parent 6357c83263
commit 643a1de09b
13 changed files with 293 additions and 55 deletions

Binary file not shown.

After

Width:  |  Height:  |  Size: 775 KiB

View File

@ -175,11 +175,15 @@ describe "Block Editor", :ignore_js_errors do
drag_and_drop_element(f(".toolbox-item.item-image"), f(".blank-section__inner"))
f(".block.image-block").click # select the section
f(".block.image-block").click # select the block
expect(block_toolbar).to be_displayed
click_block_toolbar_menu_item("Constraint", "Cover")
expect(block_resize_handle_se).to be_displayed
expect(f(".block.image-block").size.height).to eq(100)
expect(f(".block.image-block").size.width).to eq(100)
drag_and_drop_element_by(block_resize_handle_se, 100, 50)
drag_and_drop_element_by(block_resize_handle_se, 100, 0)
drag_and_drop_element_by(block_resize_handle_se, 0, 50)
expect(f(".block.image-block").size.width).to eq(200)
expect(f(".block.image-block").size.height).to eq(150)
end
@ -191,6 +195,9 @@ describe "Block Editor", :ignore_js_errors do
drag_and_drop_element(f(".toolbox-item.item-image"), f(".blank-section__inner"))
f(".block.image-block").click # select the section
f(".block.image-block").click # select the block
expect(block_toolbar).to be_displayed
click_block_toolbar_menu_item("Constraint", "Cover")
expect(block_resize_handle_se).to be_displayed
expect(f(".block.image-block").size.height).to eq(100)
expect(f(".block.image-block").size.width).to eq(100)
@ -220,6 +227,43 @@ describe "Block Editor", :ignore_js_errors do
expect(f(".block.image-block").size.width).to eq(110)
end
end
describe("resizing images that maintain aspect ratio") do
before do
@root_folder = Folder.root_folders(@course).first
@image = @root_folder.attachments.build(context: @course)
path = File.expand_path(File.dirname(__FILE__) + "/../../fixtures/block-editor/white-sands.jpg")
@image.uploaded_data = Rack::Test::UploadedFile.new(path, Attachment.mimetype(path))
@image.save!
# image is 2000w x 1000h
@block_page.update!(
block_editor_attributes: {
time: Time.now.to_i,
version: "1",
blocks: [
{
data: "{\"ROOT\":{\"type\":{\"resolvedName\":\"PageBlock\"},\"isCanvas\":true,\"props\":{},\"displayName\":\"Page\",\"custom\":{},\"hidden\":false,\"nodes\":[\"AcfL3KeXTT\"],\"linkedNodes\":{}},\"AcfL3KeXTT\":{\"type\":{\"resolvedName\":\"BlankSection\"},\"isCanvas\":false,\"props\":{},\"displayName\":\"Blank Section\",\"custom\":{\"isSection\":true},\"parent\":\"ROOT\",\"hidden\":false,\"nodes\":[],\"linkedNodes\":{\"blank-section_nosection1\":\"0ZWqBwA2Ou\"}},\"0ZWqBwA2Ou\":{\"type\":{\"resolvedName\":\"NoSections\"},\"isCanvas\":true,\"props\":{\"className\":\"blank-section__inner\"},\"displayName\":\"NoSections\",\"custom\":{\"noToolbar\":true},\"parent\":\"AcfL3KeXTT\",\"hidden\":false,\"nodes\":[\"lLVSJCBPWm\"],\"linkedNodes\":{}},\"lLVSJCBPWm\":{\"type\":{\"resolvedName\":\"ImageBlock\"},\"isCanvas\":false,\"props\":{\"src\":\"/courses/#{@course.id}/files/#{@image.id}/preview\",\"variant\":\"default\",\"constraint\":\"cover\",\"maintainAspectRatio\":true,\"width\":100,\"height\":50},\"displayName\":\"Image\",\"custom\":{\"isResizable\":true},\"parent\":\"0ZWqBwA2Ou\",\"hidden\":false,\"nodes\":[],\"linkedNodes\":{}}}"
}
]
}
)
end
it "adjusts the width when the height is changed" do
get "/courses/#{@course.id}/pages/#{@block_page.url}/edit"
wait_for_block_editor
f(".block.image-block").click # select the section
f(".block.image-block").click # select the block
expect(block_resize_handle_se).to be_displayed
expect(f(".block.image-block").size.width).to eq(100)
expect(f(".block.image-block").size.height).to eq(50)
drag_and_drop_element_by(block_resize_handle_se, 10, 50)
expect(f(".block.image-block").size.width).to eq(200)
expect(f(".block.image-block").size.height).to eq(100)
end
end
end
# rubocop:enable Specs/NoNoSuchElementError, Specs/NoExecuteScript

View File

@ -74,4 +74,13 @@ module BlockEditorPage
def block_resize_handle_se
f(".block-resizer.se")
end
def block_toolbar
f(".block-toolbar")
end
def click_block_toolbar_menu_item(menu_button_name, menu_item_name)
fj("button:contains('#{menu_button_name}')").click
fj("[role=\"menuitemcheckbox\"]:contains('#{menu_item_name}')").click
end
end

View File

@ -19,6 +19,7 @@
import React, {useCallback, useEffect, useRef, useState} from 'react'
import {useNode, type Node} from '@craftjs/core'
import {getToolbarPos as getToolbarPosUtil} from '../../utils/renderNodeHelpers'
import {getAspectRatio} from '../../utils/size'
const offset = 5
@ -68,8 +69,10 @@ const BlockResizer = ({mountPoint}: BlockResizeProps) => {
const {
actions: {setProp},
node,
maintainAspectRatio,
} = useNode((n: Node) => {
return {
maintainAspectRatio: !!node?.data?.props?.maintainAspectRatio,
node: n,
}
})
@ -117,6 +120,14 @@ const BlockResizer = ({mountPoint}: BlockResizeProps) => {
if (newWidth > 0 && newHeight > 0) {
newWidth = Math.max(newWidth, 24)
newHeight = Math.max(newHeight, 24)
if (maintainAspectRatio) {
const aspectRatio = getAspectRatio(currRect.width, currRect.height)
if (newHeight !== currRect.height) {
newWidth = newHeight * aspectRatio
} else {
newHeight = newWidth / aspectRatio
}
}
const myblock = node.dom as HTMLElement
myblock.style.width = `${newWidth}px`
myblock.style.height = `${newHeight}px`
@ -128,7 +139,7 @@ const BlockResizer = ({mountPoint}: BlockResizeProps) => {
})
}
},
[currRect.height, currRect.width, getToolbarPos, node.dom, setProp]
[currRect.height, currRect.width, getToolbarPos, maintainAspectRatio, node.dom, setProp]
)
useEffect(() => {
@ -181,13 +192,21 @@ const BlockResizer = ({mountPoint}: BlockResizeProps) => {
if (width > 0 && height > 0) {
width = Math.max(width, 24)
height = Math.max(height, 24)
if (maintainAspectRatio) {
const aspectRatio = getAspectRatio(currRect.width, currRect.height)
if (aspectRatio > 1) {
width = height * aspectRatio
} else {
height = width / aspectRatio
}
}
myblock.style.width = `${width}px`
myblock.style.height = `${height}px`
const {top, left} = getToolbarPos()
setCurrRect({left, top, width, height})
}
},
[currRect, getToolbarPos, node.dom]
[currRect, getToolbarPos, maintainAspectRatio, node.dom]
)
const handleDragEnd = useCallback(

View File

@ -16,29 +16,37 @@
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import React from 'react'
import React, {useCallback} from 'react'
import {useEditor, useNode, type Node} from '@craftjs/core'
import {Img} from '@instructure/ui-img'
import {ImageBlockToolbar} from './ImageBlockToolbar'
import {useClassNames} from '../../../../utils'
import {type ImageBlockProps, type ImageVariant, type ImageConstraint} from './types'
import {useClassNames, getAspectRatio} from '../../../../utils'
import {
EMPTY_IMAGE_WIDTH,
EMPTY_IMAGE_HEIGHT,
type ImageBlockProps,
type ImageVariant,
type ImageConstraint,
} from './types'
import {BlockResizer} from '../../../editor/BlockResizer'
import {useScope as useI18nScope} from '@canvas/i18n'
const I18n = useI18nScope('block-editor/image-block')
const ImageBlock = ({src, width, height, constraint}: ImageBlockProps) => {
const ImageBlock = ({src, width, height, constraint, maintainAspectRatio}: ImageBlockProps) => {
const {enabled} = useEditor(state => ({
enabled: state.options.enabled,
}))
const {
actions: {setProp},
connectors: {connect, drag},
node,
} = useNode((n: Node) => {
return {
selected: n.events.selected,
node: n,
}
})
const clazz = useClassNames(enabled, {empty: !src}, ['block', 'image-block'])
@ -50,6 +58,33 @@ const ImageBlock = ({src, width, height, constraint}: ImageBlockProps) => {
sty.height = `${height}px`
}
const setAspectRatio = useCallback(
(img: HTMLImageElement) => {
if (img) {
const aspectRatio = getAspectRatio(img.naturalWidth, img.naturalHeight)
if (!Number.isNaN(aspectRatio)) {
if (aspectRatio > 0) {
const newHeight = node.data.props.width / aspectRatio
setProp((props: ImageBlockProps) => (props.height = newHeight))
} else {
const newWidth = node.data.props.height * aspectRatio
setProp((props: ImageBlockProps) => (props.width = newWidth))
}
}
}
},
[node.data.props.height, node.data.props.width, setProp]
)
const handleLoad = useCallback(
(event: React.SyntheticEvent<HTMLImageElement>) => {
setAspectRatio(event.target as HTMLImageElement)
},
[setAspectRatio]
)
const imgConstrain =
(maintainAspectRatio ? 'cover' : constraint) || ImageBlock.craft.defaultProps.constraint
if (!src) {
return (
<div className={clazz} style={sty} ref={el => el && connect(drag(el as HTMLDivElement))} />
@ -60,7 +95,8 @@ const ImageBlock = ({src, width, height, constraint}: ImageBlockProps) => {
<Img
display="inline-block"
src={src || ImageBlock.craft.defaultProps.src}
constrain={constraint || ImageBlock.craft.defaultProps.constraint}
constrain={imgConstrain}
onLoad={handleLoad}
/>
</div>
)
@ -71,8 +107,11 @@ ImageBlock.craft = {
displayName: I18n.t('Image'),
defaultProps: {
src: '',
width: EMPTY_IMAGE_WIDTH,
height: EMPTY_IMAGE_HEIGHT,
variant: 'default' as ImageVariant,
constraint: 'cover' as ImageConstraint,
maintainAspectRatio: true,
},
related: {
toolbar: ImageBlockToolbar,

View File

@ -28,7 +28,12 @@ import {type ViewOwnProps} from '@instructure/ui-view'
import {UploadFileModal} from '../../../../FileUpload/UploadFileModal'
import {IconSizePopup} from './ImageSizePopup'
import {type ImageBlockProps, type ImageConstraint} from './types'
import {
EMPTY_IMAGE_WIDTH,
EMPTY_IMAGE_HEIGHT,
type ImageBlockProps,
type ImageConstraint,
} from './types'
import {useScope as useI18nScope} from '@canvas/i18n'
@ -52,8 +57,18 @@ const ImageBlockToolbar = () => {
_selected: MenuItemProps['selected'],
_args: MenuItem
) => {
const constraint = value as ImageConstraint
setProp((prps: ImageBlockProps) => (prps.constraint = constraint))
const constraint = value as ImageConstraint | 'aspect-ratio'
if (constraint === 'aspect-ratio') {
setProp((prps: ImageBlockProps) => {
prps.constraint = 'cover'
prps.maintainAspectRatio = true
})
} else {
setProp((prps: ImageBlockProps) => {
prps.constraint = constraint
prps.maintainAspectRatio = false
})
}
},
[setProp]
)
@ -82,6 +97,16 @@ const ImageBlockToolbar = () => {
[node.dom, props.height, props.width, setProp]
)
const handleChangeSz = useCallback(
(width: number, height: number) => {
setProp((prps: ImageBlockProps) => {
prps.width = width
prps.height = height
})
},
[setProp]
)
return (
<Flex gap="small">
<IconButton
@ -107,7 +132,7 @@ const ImageBlockToolbar = () => {
type="checkbox"
value="cover"
onSelect={handleConstraintChange}
selected={props.constraint === 'cover'}
selected={!props.maintainAspectRatio && props.constraint === 'cover'}
>
<Text size="small">{I18n.t('Cover')}</Text>
</Menu.Item>
@ -115,13 +140,26 @@ const ImageBlockToolbar = () => {
type="checkbox"
value="contain"
onSelect={handleConstraintChange}
selected={props.constraint === 'contain'}
selected={!props.maintainAspectRatio && props.constraint === 'contain'}
>
<Text size="small">{I18n.t('Contain')}</Text>
</Menu.Item>
<Menu.Item
type="checkbox"
value="aspect-ratio"
onSelect={handleConstraintChange}
selected={props.maintainAspectRatio}
>
<Text size="small">{I18n.t('Match Aspect Ratio')}</Text>
</Menu.Item>
</Menu>
<IconSizePopup width={props.width} height={props.height} />
<IconSizePopup
width={props.width || EMPTY_IMAGE_WIDTH}
height={props.height || EMPTY_IMAGE_HEIGHT}
maintainAspectRatio={props.maintainAspectRatio}
onChange={handleChangeSz}
/>
<UploadFileModal
imageUrl={null}

View File

@ -17,7 +17,6 @@
*/
import React, {useCallback, useEffect, useState} from 'react'
import {useNode, type Node} from '@craftjs/core'
import {FormFieldGroup} from '@instructure/ui-form-field'
import {Button, IconButton} from '@instructure/ui-buttons'
import {RangeInput} from '@instructure/ui-range-input'
@ -25,47 +24,36 @@ import {Popover} from '@instructure/ui-popover'
import {ScreenReaderContent} from '@instructure/ui-a11y-content'
import {View} from '@instructure/ui-view'
import {IconResize} from '../../../../assets/internal-icons'
import {type ImageBlockProps} from './types'
import {getAspectRatio} from '../../../../utils'
import {useScope as useI18nScope} from '@canvas/i18n'
const I18n = useI18nScope('block-editor/image-block')
type IconSizePopupProps = {
type ImageSizePopupProps = {
width: number
height: number
maintainAspectRatio: boolean
onChange: (width: number, height: number) => void
}
const IconSizePopup = ({width, height}: IconSizePopupProps) => {
const {
actions: {setProp},
domnode,
} = useNode((node: Node) => {
return {
domnode: node.dom as HTMLImageElement,
}
})
const [widthValue, setWidthValue] = useState<number>(() => {
return width || domnode.clientWidth
})
const [heightValue, setHeightValue] = useState<number>(() => {
return height || domnode.clientHeight
})
const [aspectRatio] = useState<number>(() => {
const w = width || domnode.clientWidth
const h = height || domnode.clientHeight
return w / h
})
const IconSizePopup = ({width, height, maintainAspectRatio, onChange}: ImageSizePopupProps) => {
const [widthValue, setWidthValue] = useState<number>(width)
const [heightValue, setHeightValue] = useState<number>(height)
const [aspectRatio, setAspectRatio] = useState<number>(getAspectRatio(width, height))
const [isShowingContent, setIsShowingContent] = useState(false)
useEffect(() => {
setWidthValue(width || domnode.clientWidth)
setHeightValue(height || domnode.clientHeight)
}, [width, height, domnode.clientWidth, domnode.clientHeight])
setWidthValue(width)
setHeightValue(height)
setAspectRatio(getAspectRatio(width, height))
}, [width, height])
const handleShowContent = useCallback(() => {
setWidthValue(width)
setHeightValue(height)
setIsShowingContent(true)
}, [])
}, [height, width])
const handleHideContent = useCallback(() => {
setIsShowingContent(false)
@ -74,30 +62,31 @@ const IconSizePopup = ({width, height}: IconSizePopupProps) => {
const handleChangeWidth = useCallback(
(value: number | string) => {
const w = typeof value === 'number' ? value : parseInt(value, 10)
const h = Math.round(w / aspectRatio)
setWidthValue(w)
setHeightValue(h)
if (maintainAspectRatio) {
const h = Math.round(w / aspectRatio)
setHeightValue(h)
}
},
[aspectRatio]
[aspectRatio, maintainAspectRatio]
)
const handleChangeHeight = useCallback(
(value: number | string) => {
const h = typeof value === 'number' ? value : parseInt(value, 10)
setHeightValue(h)
const w = Math.round(h * aspectRatio)
setWidthValue(w)
if (maintainAspectRatio) {
const w = Math.round(h * aspectRatio)
setWidthValue(w)
}
},
[aspectRatio]
[aspectRatio, maintainAspectRatio]
)
const setImageSize = useCallback(() => {
setProp((prps: ImageBlockProps) => {
prps.width = widthValue
prps.height = heightValue
})
onChange(widthValue, heightValue)
setIsShowingContent(false)
}, [heightValue, setProp, widthValue])
}, [heightValue, onChange, widthValue])
return (
<Popover

View File

@ -55,8 +55,22 @@ describe('ImageBlock', () => {
expect(img).toHaveStyle({objectFit: 'cover'})
})
it('should render with cover constraint when maintainAspectRatio is true, regardless of constraint prop', () => {
const {container} = renderBlock({
src: 'https://example.com/image.jpg',
constraint: 'contain',
maintainAspectRatio: true,
})
const img = container.querySelector('img')
expect(img).toHaveStyle({objectFit: 'cover'})
})
it('should render with contain constraint', () => {
const {container} = renderBlock({src: 'https://example.com/image.jpg', constraint: 'contain'})
const {container} = renderBlock({
src: 'https://example.com/image.jpg',
constraint: 'contain',
maintainAspectRatio: false,
})
const img = container.querySelector('img')
expect(img).toHaveStyle({objectFit: 'contain'})
})

View File

@ -70,11 +70,13 @@ describe('ImageBlockToolbar', () => {
const coverMenuItem = screen.getByText('Cover')
const containMenuItem = screen.getByText('Contain')
const aspectRatioMenuItem = screen.getByText('Match Aspect Ratio')
expect(coverMenuItem).toBeInTheDocument()
expect(containMenuItem).toBeInTheDocument()
expect(aspectRatioMenuItem).toBeInTheDocument()
const li = coverMenuItem.closest('li') as HTMLLIElement
const li = aspectRatioMenuItem.closest('li') as HTMLLIElement
expect(li.querySelector('svg[name="IconCheck"]')).toBeInTheDocument()
})
@ -89,6 +91,29 @@ describe('ImageBlockToolbar', () => {
expect(mockSetProp).toHaveBeenCalled()
expect(props.constraint).toBe('contain')
expect(props.maintainAspectRatio).toBe(false)
})
it('changes the maintainAspectRatio prop', async () => {
props.maintainAspectRatio = false
const {getByText} = render(<ImageBlockToolbar />)
const btn = getByText('Constraint').closest('button') as HTMLButtonElement
await user.click(btn)
const coverMenuItem = screen.getByText('Cover')
let li = coverMenuItem.closest('li') as HTMLLIElement
expect(li.querySelector('svg[name="IconCheck"]')).toBeInTheDocument()
const aspectRatioMenuItem = screen.getByText('Match Aspect Ratio')
li = aspectRatioMenuItem.closest('li') as HTMLLIElement
expect(li.querySelector('svg[name="IconCheck"]')).not.toBeInTheDocument()
await user.click(aspectRatioMenuItem)
expect(mockSetProp).toHaveBeenCalled()
expect(props.maintainAspectRatio).toBe(true)
expect(props.constraint).toBe('cover')
})
it('shows the size popup', async () => {

View File

@ -25,4 +25,8 @@ export type ImageBlockProps = {
width?: number
height?: number
constraint?: ImageConstraint
maintainAspectRatio?: boolean
}
export const EMPTY_IMAGE_WIDTH = 100
export const EMPTY_IMAGE_HEIGHT = 100

View File

@ -0,0 +1,33 @@
/*
* Copyright (C) 2024 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
import {getAspectRatio} from '../size'
describe('getAspectRatio', () => {
it('should return the aspect ratio of the width and height', () => {
expect(getAspectRatio(100, 100)).toEqual(1)
expect(getAspectRatio(200, 100)).toEqual(2)
expect(getAspectRatio(100, 200)).toEqual(0.5)
})
it('should return 1 if the aspect ratio is not a number or not finite', () => {
expect(getAspectRatio(100, 0)).toEqual(1)
expect(getAspectRatio(Number.NaN, 100)).toEqual(1)
expect(getAspectRatio(0, 0)).toEqual(1)
})
})

View File

@ -25,3 +25,4 @@ export * from './getCloneTree'
export * from './colorUtils'
export * from './getNodeIndex'
export * from './deletable'
export * from './size'

View File

@ -0,0 +1,23 @@
/*
* Copyright (C) 2024 - present Instructure, Inc.
*
* This file is part of Canvas.
*
* Canvas is free software: you can redistribute it and/or modify it under
* the terms of the GNU Affero General Public License as published by the Free
* Software Foundation, version 3 of the License.
*
* Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
* A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
* details.
*
* You should have received a copy of the GNU Affero General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
export const getAspectRatio = (width: number, height: number) => {
let ar = width / height
if (Number.isNaN(ar) || !Number.isFinite(ar)) ar = 1
return ar
}