Fix sizing if ImageBlock

closes RCX-2585
flag=block_editor

test plan:
  - create 2 columns
  - make one column tall (add a slew of icons?)
  - add an image to the other column
  - set size to percent and drag to make 100% tall
  > expect the image to fill the column
  - change among auto, pixel, and percent
  - resize the image
  > expect it to behave
  - look at the preview
  > expect it to behave

Change-Id: I9107dc07aea4de83cdb9c2f8b2a04b6e00b5a684
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/360874
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Eric Saupe <eric.saupe@instructure.com>
QA-Review: Eric Saupe <eric.saupe@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>

ui/shared/block-editor/react/components/editor/BlockResizer.tsx

@@ -18,20 +18,17 @@
 
 import React, {useCallback, useEffect, useRef, useState} from 'react'
 import {useNode, type Node} from '@craftjs/core'
-import {getAspectRatio} from '../../utils/size'
-import {type ResizableProps} from './types'
 import Moveable, {type OnResize} from 'react-moveable'
-
-export type Sz = {
-  width: number
-  height: number
-}
+import {getAspectRatio, percentSize} from '../../utils'
+import {type ResizableProps, type SizeVariant, type Sz} from './types'
+import {px} from '@instructure/ui-utils'
 
 type BlockResizeProps = {
   mountPoint: HTMLElement
+  sizeVariant: SizeVariant
 }
 
-const BlockResizer = ({mountPoint}: BlockResizeProps) => {
+const BlockResizer = ({mountPoint, sizeVariant}: BlockResizeProps) => {
   const {
     actions: {setProp},
     maintainAspectRatio,
@@ -70,16 +67,34 @@ const BlockResizer = ({mountPoint}: BlockResizeProps) => {
       myblock.style.height = `${newHeight}px`
 
       setCurrSz({width: newWidth, height: newHeight})
+
+      let propWidth = newWidth,
+        propHeight = newHeight
+      if (sizeVariant === 'percent') {
+        const parent = node.dom?.offsetParent
+        if (parent) {
+          // assume all 4 sides have the same padding
+          const padding = px(window.getComputedStyle(parent).getPropertyValue('padding'))
+          const {width, height} = percentSize(
+            parent.clientWidth - padding,
+            parent.clientHeight - padding,
+            newWidth,
+            newHeight
+          )
+          propWidth = width
+          propHeight = height
+        }
+      }
       setProp((props: any) => {
-        props.width = newWidth
-        props.height = newHeight
+        props.width = propWidth
+        props.height = propHeight
       })
     },
-    [currSz.height, currSz.width, maintainAspectRatio, node.dom, setProp]
+    [currSz.height, currSz.width, maintainAspectRatio, node.dom, setProp, sizeVariant]
   )
 
   const handleResizeKeys = useCallback(
-    event => {
+    (event: KeyboardEvent) => {
       if (!node.dom) return
       if (!event.altKey) return
 

ui/shared/block-editor/react/components/editor/RenderNode.tsx

@@ -159,7 +159,10 @@ export const RenderNode: RenderNodeComponent = ({render}: RenderNodeProps) => {
   const renderResizer = () => {
     if (!mountPoint) return null
 
-    return ReactDOM.createPortal(<BlockResizer mountPoint={mountPoint} />, mountPoint)
+    return ReactDOM.createPortal(
+      <BlockResizer mountPoint={mountPoint} sizeVariant={node.data.props.sizeVariant || 'pixel'} />,
+      mountPoint
+    )
   }
 
   const renderRelated = () => {

ui/shared/block-editor/react/components/editor/types.ts

@@ -26,6 +26,11 @@ export interface RenderNodeProps {
 export type AddSectionPlacement = 'prepend' | 'append' | undefined
 export type SizeVariant = 'auto' | 'pixel' | 'percent'
 
+export type Sz = {
+  width: number
+  height: number
+}
+
 export type ResizableProps = {
   width?: number
   height?: number

ui/shared/block-editor/react/components/user/blocks/ImageBlock/ImageBlock.tsx

@@ -16,11 +16,8 @@
  * with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-import React, {CSSProperties, useCallback, useEffect, useRef, useState} from 'react'
-import {useEditor, useNode, type Node} from '@craftjs/core'
-
-import {Img} from '@instructure/ui-img'
-
+import React, {type CSSProperties, useCallback, useEffect, useRef, useState} from 'react'
+import {useEditor, useNode} from '@craftjs/core'
 import {ImageBlockToolbar} from './ImageBlockToolbar'
 import {useClassNames} from '../../../../utils'
 import {type ImageBlockProps, type ImageVariant, type ImageConstraint} from './types'
@@ -44,26 +41,22 @@ const ImageBlock = ({
     enabled: state.options.enabled,
   }))
   const {
-    actions: {setProp, setCustom},
+    actions: {setCustom},
     connectors: {connect, drag},
-  } = useNode((n: Node) => {
-    return {
-      node: n,
-    }
-  })
+  } = useNode()
   const clazz = useClassNames(enabled, {empty: !src}, ['block', 'image-block'])
   const [styl, setStyl] = useState<any>({})
   const [imageLoaded, setImageLoaded] = useState(false)
-  const [currSzVariant, setCurrSzVariant] = useState(sizeVariant)
-  const [currKeepAR, setCurrKeepAR] = useState(maintainAspectRatio)
   const [aspectRatio, setAspectRatio] = useState(1)
   // in preview mode, node.dom is null, so use a ref to the element
   const [blockRef, setBlockRef] = useState<HTMLDivElement | null>(null)
   const imgRef = useRef<HTMLImageElement | null>(null)
   const loadingStyle = {
     position: 'absolute',
-    left: 'calc(50% - 24px)',
-    top: 'calc(50% - 24px)',
+    left: '10px',
+    top: '10px',
+    width: '100px',
+    height: '100px',
   } as CSSProperties
 
   const setSize = useCallback(() => {
@@ -77,19 +70,14 @@ const ImageBlock = ({
       return
     }
     const sty: any = {}
+    const unit = sizeVariant === 'percent' ? '%' : 'px'
     if (width) {
-      if (sizeVariant === 'percent') {
-        const parent = blockRef.offsetParent
-        const pctw = parent ? (width / parent.clientWidth) * 100 : 100
-        sty.width = `${pctw}%`
-      } else {
-        sty.width = `${width}px`
-      }
+      sty.width = `${width}${unit}`
     }
     if (maintainAspectRatio) {
       sty.height = 'auto'
-    } else if (sizeVariant === 'pixel' || sizeVariant === 'percent') {
-      sty.height = `${height}px`
+    } else {
+      sty.height = `${height}${unit}`
     }
     setStyl(sty)
   }, [blockRef, height, maintainAspectRatio, sizeVariant, src, width])
@@ -104,37 +92,17 @@ const ImageBlock = ({
       const img = imgRef.current
       setImageLoaded(true)
       setAspectRatio(img.naturalWidth / img.naturalHeight)
-      setProp((props: any) => {
-        props.width = img.width
-        props.height = img.height
-      })
       clearInterval(loadTimer)
     }, 10)
     return () => {
       clearInterval(loadTimer)
     }
-  }, [imageLoaded, setProp, src])
+  }, [imageLoaded, src])
 
   useEffect(() => {
     setSize()
   }, [width, height, aspectRatio, setSize])
 
-  useEffect(() => {
-    if (currSzVariant !== sizeVariant || currKeepAR !== maintainAspectRatio) {
-      setCurrSzVariant(sizeVariant)
-      setCurrKeepAR(maintainAspectRatio)
-      setSize()
-      if (!maintainAspectRatio) {
-        setProp((props: any) => {
-          if (imgRef.current) {
-            props.width = imgRef.current.clientWidth
-            props.height = imgRef.current.clientHeight
-          }
-        })
-      }
-    }
-  }, [currKeepAR, currSzVariant, maintainAspectRatio, setProp, setSize, sizeVariant])
-
   useEffect(() => {
     setCustom((ctsm: any) => {
       ctsm.isResizable = !!src && sizeVariant !== 'auto'
@@ -161,29 +129,24 @@ const ImageBlock = ({
         aria-label={ImageBlock.craft.displayName}
         tabIndex={-1}
         className={clazz}
-        style={styl}
+        style={{...styl, position: 'relative'}}
         ref={el => {
           el && connect(drag(el as HTMLDivElement))
           setBlockRef(el)
         }}
       >
-        <div style={{position: 'relative'}}>
-          {!imgRef?.current?.complete ? (
-            <div style={loadingStyle}>
-              <Spinner renderTitle={I18n.t('Loading')} size="small" />
-            </div>
-          ) : null}
-          <div style={!imgRef?.current?.complete ? {opacity: '0.2'} : {}}>
-            <Img
-              elementRef={el => (imgRef.current = el as HTMLImageElement)}
-              display="inline-block"
-              src={src || ImageBlock.craft.defaultProps.src}
-              constrain={imgConstrain}
-              alt={alt || ''}
-              style={styl}
-            />
+        {!imgRef?.current?.complete ? (
+          <div style={loadingStyle}>
+            <Spinner renderTitle={I18n.t('Loading')} size="x-small" />
           </div>
-        </div>
+        ) : null}
+
+        <img
+          ref={imgRef}
+          src={src || ImageBlock.craft.defaultProps.src}
+          alt={alt || ''}
+          style={{width: '100%', height: '100%', objectFit: imgConstrain, display: 'inline-block'}}
+        />
       </div>
     )
   }

ui/shared/block-editor/react/components/user/blocks/ImageBlock/ImageBlockToolbar.tsx

@@ -34,20 +34,24 @@ import {useScope as useI18nScope} from '@canvas/i18n'
 import {Popover} from '@instructure/ui-popover'
 import {TextArea} from '@instructure/ui-text-area'
 
+import {changeSizeVariant} from '../../../../utils/resizeHelpers'
+
 const I18n = useI18nScope('block-editor/image-block')
 
 const ImageBlockToolbar = () => {
   const {
     actions: {setProp},
+    node,
     props,
   } = useNode((n: Node) => ({
+    node: n,
     props: n.data.props,
   }))
   const [showUploadModal, setShowUploadModal] = useState(false)
 
   const handleConstraintChange = useCallback(
     (
-      e: React.MouseEvent<ViewOwnProps, MouseEvent>,
+      _e: React.MouseEvent<ViewOwnProps, MouseEvent>,
       value: MenuItemProps['value'] | MenuItemProps['value'][],
       _selected: MenuItemProps['selected'],
       _args: MenuItem
@@ -70,16 +74,22 @@ const ImageBlockToolbar = () => {
 
   const handleChangeSzVariant = useCallback(
     (
-      e: React.MouseEvent<ViewOwnProps, MouseEvent>,
+      _e: React.MouseEvent<ViewOwnProps, MouseEvent>,
       value: MenuItemProps['value'] | MenuItemProps['value'][],
       _selected: MenuItemProps['selected'],
       _args: MenuItem
     ) => {
       setProp((prps: ImageBlockProps) => {
         prps.sizeVariant = value as SizeVariant
+
+        if (node.dom) {
+          const {width, height} = changeSizeVariant(node.dom, value as SizeVariant)
+          prps.width = width
+          prps.height = height
+        }
       })
     },
-    [setProp]
+    [node.dom, setProp]
   )
 
   const handleShowUploadModal = useCallback(() => {

ui/shared/block-editor/react/components/user/blocks/ImageBlock/__tests__/ImageBlock.test.tsx

@@ -73,8 +73,7 @@ describe('ImageBlock', () => {
         height: 201,
       })
       const img = container.querySelector('.image-block') as HTMLElement
-      expect(img).toHaveStyle({height: '201px'})
-      expect(img.style.width).toMatch(/%$/)
+      expect(img).toHaveStyle({height: '201%', width: '101%'})
     })
 
     it('should render %width and auto height with "percent" sizeVariant and maintainAspectRatio', () => {

ui/shared/block-editor/react/utils/__tests__/resizeHelpers.test.ts

@@ -0,0 +1,62 @@
+/*
+ * 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 {changeSizeVariant, percentSize} from '../resizeHelpers'
+
+describe('resizeHelpers', () => {
+  describe('changeSizeVariant', () => {
+    it('returns the current size of the element if the new size variant is auto', () => {
+      const elem = {} as HTMLElement
+      elem.getBoundingClientRect = jest.fn().mockReturnValue({width: 100, height: 100})
+      expect(changeSizeVariant(elem, 'auto')).toEqual({width: 100, height: 100})
+    })
+
+    it('returns the current size of the element if the new size variant is pixel', () => {
+      const elem = {} as HTMLElement
+      elem.getBoundingClientRect = jest.fn().mockReturnValue({width: 100, height: 100})
+      expect(changeSizeVariant(elem, 'pixel')).toEqual({width: 100, height: 100})
+    })
+
+    it('returns the current size of the element as a percentage of the parent if the new size variant is percent', () => {
+      const parent = {
+        clientWidth: 200,
+        clientHeight: 200,
+      } as HTMLElement
+      const elem = {} as HTMLElement
+      // @ts-expect-error
+      elem.offsetParent = parent
+      elem.getBoundingClientRect = jest.fn().mockReturnValue({width: 100, height: 100})
+
+      expect(changeSizeVariant(elem, 'percent')).toEqual({width: 50, height: 50})
+    })
+  })
+
+  describe('percentSize', () => {
+    it('returns the percentage of the parent width and height that the element occupies', () => {
+      expect(percentSize(200, 200, 100, 100)).toEqual({width: 50, height: 50})
+    })
+
+    it('returns 100% if the element is within 7px of the parent width', () => {
+      expect(percentSize(200, 200, 193, 100)).toEqual({width: 100, height: 50})
+    })
+
+    it('returns 100% if the element is within 7px of the parent height', () => {
+      expect(percentSize(200, 200, 100, 193)).toEqual({width: 50, height: 100})
+    })
+  })
+})

ui/shared/block-editor/react/utils/index.ts

@@ -34,3 +34,4 @@ export * from './mergeTemplates'
 export * from './saveGlobalTemplate'
 export * from './deletable'
 export * from './getTemplates'
+export * from './resizeHelpers'

ui/shared/block-editor/react/utils/resizeHelpers.ts

@@ -0,0 +1,47 @@
+/*
+ * 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 {type SizeVariant, type Sz} from '../components/editor/types'
+
+const changeSizeVariant = (elem: HTMLElement, to: SizeVariant): Sz => {
+  const {width, height} = elem.getBoundingClientRect()
+  if (to === 'percent') {
+    const parent = elem.offsetParent
+    if (parent) {
+      return percentSize(parent.clientWidth, parent.clientHeight, width, height)
+    }
+  }
+  return {width, height}
+}
+
+const percentSize = (parentWidth: number, parentHeight: number, width: number, height: number) => {
+  let w = width,
+    h = height
+  if (parentWidth - width <= 7) {
+    w = parentWidth
+  }
+  if (parentHeight - height <= 7) {
+    h = parentHeight
+  }
+  return {
+    width: (w / parentWidth) * 100,
+    height: (h / parentHeight) * 100,
+  }
+}
+
+export {changeSizeVariant, percentSize}