Skip to content

Commit

Permalink
Pan to CB (#9273)
Browse files Browse the repository at this point in the history
When the CB is opened, pan to show it.

Large screen:
<video src="https://github.com/enso-org/enso/assets/1047859/1a07c8cc-5818-420a-9fb3-1d1cb308cb87">

Small screen:
<video src="https://github.com/enso-org/enso/assets/1047859/a9f18df5-c0ca-426c-959a-bda5cd077541">

# Important Notes
A prioritized-coordinates approach is used to adjust panning goals based on screen space:
- Fitting the input area is highest-priority.
- If possible, the whole component panel area will be fit.
- If possible, the visualization preview will be fit.
- If there's extra room, margins will be included; the top and left are prioritized because those margins prevent overlap with fixed UI elements.
kazcw authored Mar 6, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent e930738 commit 79a6a6a
Showing 6 changed files with 179 additions and 17 deletions.
68 changes: 51 additions & 17 deletions app/gui2/e2e/componentBrowser.spec.ts
Original file line number Diff line number Diff line change
@@ -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 }) => {
2 changes: 2 additions & 0 deletions app/gui2/mock/providers.ts
Original file line number Diff line number Diff line change
@@ -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: '',
46 changes: 46 additions & 0 deletions app/gui2/src/components/ComponentBrowser.vue
Original file line number Diff line number Diff line change
@@ -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<T extends Record<any, number>>(
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 ===
17 changes: 17 additions & 0 deletions app/gui2/src/composables/navigator.ts
Original file line number Diff line number Diff line change
@@ -93,6 +93,21 @@ export function useNavigator(viewportNode: Ref<Element | undefined>) {
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<Vec2>[]) {
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<Element | undefined>) {
},
},
translate,
targetScale,
scale,
viewBox,
transform,
@@ -234,6 +250,7 @@ export function useNavigator(viewportNode: Ref<Element | undefined>) {
clientToScenePos,
clientToSceneRect,
panAndZoomTo,
panTo,
viewport,
})
}
37 changes: 37 additions & 0 deletions app/gui2/src/util/data/__tests__/rect.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
26 changes: 26 additions & 0 deletions app/gui2/src/util/data/rect.ts
Original file line number Diff line number Diff line change
@@ -68,6 +68,13 @@ export class Rect {
)
}

contains(coord: Partial<Vec2>): 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<Vec2>): 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)

0 comments on commit 79a6a6a

Please sign in to comment.