diff --git a/app/gui2/e2e/componentBrowser.spec.ts b/app/gui2/e2e/componentBrowser.spec.ts index 4cb923e20186..ed98de9040eb 100644 --- a/app/gui2/e2e/componentBrowser.spec.ts +++ b/app/gui2/e2e/componentBrowser.spec.ts @@ -13,38 +13,39 @@ async function deselectAllNodes(page: Page) { await expect(page.locator('.GraphNode.selected')).toHaveCount(0) } -test('Different ways of opening Component Browser', async ({ page }) => { - await actions.goToGraph(page) +async function expectAndCancelBrowser(page: Page, expectedInput: string) { const nodeCount = await locate.graphNode(page).count() + await expect(locate.componentBrowser(page)).toExist() + await expect(locate.componentBrowserEntry(page)).toExist() + await expect(locate.componentBrowserInput(page).locator('input')).toHaveValue(expectedInput) + await expect(locate.componentBrowserInput(page).locator('input')).toBeInViewport() + await page.keyboard.press('Escape') + await expect(locate.componentBrowser(page)).not.toBeVisible() + await expect(locate.graphNode(page)).toHaveCount(nodeCount) +} - async function expectAndCancelBrowser(expectedInput: string) { - await expect(locate.componentBrowser(page)).toExist() - await expect(locate.componentBrowserEntry(page)).toExist() - await expect(locate.componentBrowserInput(page).locator('input')).toHaveValue(expectedInput) - await page.keyboard.press('Escape') - await expect(locate.componentBrowser(page)).not.toBeVisible() - await expect(locate.graphNode(page)).toHaveCount(nodeCount) - } +test('Different ways of opening Component Browser', async ({ page }) => { + await actions.goToGraph(page) // Without source node // (+) button await locate.addNewNodeButton(page).click() - await expectAndCancelBrowser('') + await expectAndCancelBrowser(page, '') // Enter key await locate.graphEditor(page).press('Enter') - await expectAndCancelBrowser('') + await expectAndCancelBrowser(page, '') // With source node // (+) button await locate.graphNodeByBinding(page, 'final').click() await locate.addNewNodeButton(page).click() - await expectAndCancelBrowser('final.') + await expectAndCancelBrowser(page, 'final.') // Enter key await locate.graphNodeByBinding(page, 'final').click() await locate.graphEditor(page).press('Enter') - await expectAndCancelBrowser('final.') + await expectAndCancelBrowser(page, 'final.') // Dragging out an edge // `click` method of locator could be simpler, but `position` option doesn't work. const outputPortArea = await locate @@ -55,15 +56,48 @@ test('Different ways of opening Component Browser', async ({ page }) => { const outputPortX = outputPortArea.x + outputPortArea.width / 2.0 const outputPortY = outputPortArea.y + outputPortArea.height - 2.0 await page.mouse.click(outputPortX, outputPortY) - await page.mouse.click(40, 300) - await expectAndCancelBrowser('final.') + await page.mouse.click(100, 500) + await expectAndCancelBrowser(page, 'final.') // Double-clicking port // TODO[ao] Without timeout, even the first click would be treated as double due to previous // event. Probably we need a better way to simulate double clicks. await page.waitForTimeout(600) await page.mouse.click(outputPortX, outputPortY) await page.mouse.click(outputPortX, outputPortY) - await expectAndCancelBrowser('final.') + await expectAndCancelBrowser(page, 'final.') +}) + +test('Graph Editor pans to Component Browser', async ({ page }) => { + await actions.goToGraph(page) + + // Select node, pan out of view of it, press Enter; should pan to show node and CB + await locate.graphNodeByBinding(page, 'final').click() + await page.mouse.move(100, 80) + await page.mouse.down({ button: 'middle' }) + await page.mouse.move(100, 700) + await page.mouse.up({ button: 'middle' }) + await expect(locate.graphNodeByBinding(page, 'final')).not.toBeInViewport() + await locate.graphEditor(page).press('Enter') + await expect(locate.graphNodeByBinding(page, 'final')).toBeInViewport() + await expectAndCancelBrowser(page, 'final.') + + // Dragging out an edge to the bottom of the viewport; when the CB pans into view, some nodes are out of view. + await page.mouse.move(100, 1100) + await page.mouse.down({ button: 'middle' }) + await page.mouse.move(100, 80) + await page.mouse.up({ button: 'middle' }) + await expect(locate.graphNodeByBinding(page, 'five')).toBeInViewport() + const outputPortArea = await locate + .graphNodeByBinding(page, 'final') + .locator('.outputPortHoverArea') + .boundingBox() + assert(outputPortArea) + const outputPortX = outputPortArea.x + outputPortArea.width / 2.0 + const outputPortY = outputPortArea.y + outputPortArea.height - 2.0 + await page.mouse.click(outputPortX, outputPortY) + await page.mouse.click(100, 1550) + await expect(locate.graphNodeByBinding(page, 'five')).not.toBeInViewport() + await expectAndCancelBrowser(page, 'final.') }) test('Accepting suggestion', async ({ page }) => { diff --git a/app/gui2/mock/providers.ts b/app/gui2/mock/providers.ts index 9845731a5550..2a41f60057bb 100644 --- a/app/gui2/mock/providers.ts +++ b/app/gui2/mock/providers.ts @@ -8,9 +8,11 @@ export const graphNavigator: GraphNavigator = { clientToScenePos: () => Vec2.Zero, clientToSceneRect: () => Rect.Zero, panAndZoomTo: () => {}, + panTo: () => {}, transform: '', prescaledTransform: '', translate: Vec2.Zero, + targetScale: 1, scale: 1, sceneMousePos: Vec2.Zero, viewBox: '', diff --git a/app/gui2/src/components/ComponentBrowser.vue b/app/gui2/src/components/ComponentBrowser.vue index 09b19452c5eb..5045b3db7fac 100644 --- a/app/gui2/src/components/ComponentBrowser.vue +++ b/app/gui2/src/components/ComponentBrowser.vue @@ -22,6 +22,7 @@ import { targetIsOutside } from '@/util/autoBlur' import { tryGetIndex } from '@/util/data/array' import type { Opt } from '@/util/data/opt' import { allRanges } from '@/util/data/range' +import { Rect } from '@/util/data/rect' import { Vec2 } from '@/util/data/vec2' import { debouncedGetter } from '@/util/reactivity' import type { SuggestionId } from 'shared/languageServerTypes/suggestions' @@ -33,6 +34,17 @@ const TOP_BAR_HEIGHT = 32 // Difference in position between the component browser and a node for the input of the component browser to // be placed at the same position as the node. const COMPONENT_BROWSER_TO_NODE_OFFSET = new Vec2(-4, -4) +const WIDTH = 600 +const INPUT_AREA_HEIGHT = 40 +const PANELS_HEIGHT = 384 +// Height of the visualization area, starting from the bottom of the input area. +const VISUALIZATION_HEIGHT = 190 +const PAN_MARGINS = { + top: 48, + bottom: 40, + left: 80, + right: 40, +} const projectStore = useProjectStore() const suggestionDbStore = useSuggestionDbStore() @@ -66,6 +78,39 @@ const cbOpen: Interaction = { }, } +function scaleValues>( + values: T, + scale: number, +): { [Key in keyof T]: number } { + return Object.fromEntries( + Object.entries(values).map(([key, value]) => [key, value * scale]), + ) as any +} + +function panIntoView() { + // Factor that converts client-coordinate dimensions to scene-coordinate dimensions. + const scale = 1 / props.navigator.targetScale + const origin = props.nodePosition.add(COMPONENT_BROWSER_TO_NODE_OFFSET.scale(scale)) + const inputArea = new Rect(origin, new Vec2(WIDTH, INPUT_AREA_HEIGHT).scale(scale)) + const panelsAreaDimensions = new Vec2(WIDTH, PANELS_HEIGHT).scale(scale) + const panelsArea = new Rect(origin.sub(new Vec2(0, panelsAreaDimensions.y)), panelsAreaDimensions) + const vizHeight = VISUALIZATION_HEIGHT * scale + const margins = scaleValues(PAN_MARGINS, scale) + props.navigator.panTo([ + // Always include the bottom-left of the input area. + { x: inputArea.left, y: inputArea.bottom }, + // Try to reach the top-right corner of the panels. + { x: inputArea.right, y: panelsArea.top }, + // Extend down to include the visualization. + { y: inputArea.bottom + vizHeight }, + // Top (and left) margins are more important than bottom (and right) margins because the screen has controls across + // the top and on the left. + { x: inputArea.left - margins.left, y: panelsArea.top - margins.top }, + // If the screen is very spacious, even the bottom right gets some breathing room. + { x: inputArea.right + margins.right, y: inputArea.bottom + vizHeight + margins.bottom }, + ]) +} + onMounted(() => { interaction.setCurrent(cbOpen) input.reset(props.usage) @@ -76,6 +121,7 @@ onMounted(() => { 'Component Browser input element was not mounted. This is not expected and may break the Component Browser', ) } + panIntoView() }) // === Position === diff --git a/app/gui2/src/composables/navigator.ts b/app/gui2/src/composables/navigator.ts index 17e14793264e..c070567d9ad0 100644 --- a/app/gui2/src/composables/navigator.ts +++ b/app/gui2/src/composables/navigator.ts @@ -93,6 +93,21 @@ export function useNavigator(viewportNode: Ref) { targetCenter.value = new Vec2(centerX, centerY) } + /** Pan to include the given prioritized list of coordinates. + * + * The view will be offset to include each coordinate, unless the coordinate cannot be fit in the viewport without + * losing a previous (higher-priority) coordinate; in that case, shift the viewport as close as possible to the + * coordinate while still satisfying the more important constraints. + * + * If all provided constraints can be met, the viewport will be moved the shortest distance that fits all the + * coordinates in view. + */ + function panTo(points: Partial[]) { + let target = viewport.value + for (const point of points.reverse()) target = target.offsetToInclude(point) ?? target + targetCenter.value = target.center() + } + let zoomPivot = Vec2.Zero const zoomPointer = usePointer((pos, _event, ty) => { if (ty === 'start') { @@ -225,6 +240,7 @@ export function useNavigator(viewportNode: Ref) { }, }, translate, + targetScale, scale, viewBox, transform, @@ -234,6 +250,7 @@ export function useNavigator(viewportNode: Ref) { clientToScenePos, clientToSceneRect, panAndZoomTo, + panTo, viewport, }) } diff --git a/app/gui2/src/util/data/__tests__/rect.test.ts b/app/gui2/src/util/data/__tests__/rect.test.ts new file mode 100644 index 000000000000..e5baf46d6473 --- /dev/null +++ b/app/gui2/src/util/data/__tests__/rect.test.ts @@ -0,0 +1,37 @@ +import { Rect } from '@/util/data/rect' +import { Vec2 } from '@/util/data/vec2' +import { fc, test } from '@fast-check/vitest' +import { expect } from 'vitest' + +test.prop({ + rectX: fc.nat(), + rectY: fc.nat(), + width: fc.nat(), + height: fc.nat(), + x: fc.nat(), + y: fc.nat(), +})('offsetToInclude', ({ rectX, rectY, width, height, x, y }) => { + const rect = new Rect(new Vec2(rectX, rectY), new Vec2(width, height)) + const point = new Vec2(x, y) + const offsetRect = rect.offsetToInclude(point) + expect( + offsetRect === undefined, + '`offsetToInclude` returns `undefined` iffi the original `Rect` contains the point.', + ).toBe(rect.contains(point)) + if (offsetRect === undefined) return + expect( + offsetRect.size === rect.size, + 'The result of `offsetToInclude` is the same size as the input `Rect`.', + ) + expect(offsetRect.contains(point), 'The result of `offsetToInclude` contains the point.') + const dx = Math.max(0, rect.left - point.x, point.x - rect.right) + const dy = Math.max(0, rect.top - point.y, point.y - rect.bottom) + expect( + Math.abs(offsetRect.left - rect.left), + '`offsetToInclude` has shifted the `Rect` by the minimum distance that reaches the point on the x-axis.', + ).toBe(dx) + expect( + Math.abs(offsetRect.top - rect.top), + '`offsetToInclude` has shifted the `Rect` by the minimum distance that reaches the point on the y-axis.', + ).toBe(dy) +}) diff --git a/app/gui2/src/util/data/rect.ts b/app/gui2/src/util/data/rect.ts index fdc5e38ad3f0..66e79b1fb8d5 100644 --- a/app/gui2/src/util/data/rect.ts +++ b/app/gui2/src/util/data/rect.ts @@ -68,6 +68,13 @@ export class Rect { ) } + contains(coord: Partial): boolean { + return ( + (coord.x == null || (this.left <= coord.x && this.right >= coord.x)) && + (coord.y == null || (this.top <= coord.y && this.bottom >= coord.y)) + ) + } + center(): Vec2 { return this.pos.addScaled(this.size, 0.5) } @@ -87,6 +94,25 @@ export class Rect { intersects(other: Rect): boolean { return this.intersectsX(other) && this.intersectsY(other) } + + /** If this `Rect` already includes `coord`, return `undefined`; otherwise, return a new `Rect` that has been shifted + * by the minimum distance that causes it to include the coordinate. The coordinate may be a point or may specify + * only an `x` or `y` bound to leave the other dimension unchanged. + */ + offsetToInclude(coord: Partial): Rect | undefined { + const newX = + coord.x == null ? undefined + : coord.x < this.left ? coord.x + : coord.x > this.right ? coord.x - this.width + : undefined + const newY = + coord.y == null ? undefined + : coord.y < this.top ? coord.y + : coord.y > this.bottom ? coord.y - this.height + : undefined + if (newX == null && newY == null) return + return new Rect(new Vec2(newX ?? this.pos.x, newY ?? this.pos.y), this.size) + } } Rect.Zero = new Rect(Vec2.Zero, Vec2.Zero)