Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[v9] refactor!: remove updateStyle from Size #2489

Merged
merged 2 commits into from
Sep 9, 2022
Merged

[v9] refactor!: remove updateStyle from Size #2489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
efbaac8
refactor!: remove `updateStyle` from `Size`
CodyJasonBennett Sep 9, 2022
201ec47
fix: don't override size w/defaults
CodyJasonBennett Sep 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/API/hooks.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ The hook is reactive, if you resize the browser for instance, you get fresh meas
| legacy | Disables global color management via `THREE.ColorManagement` | `boolean` |
| frameloop | Render mode: always, demand, never | `always`, `demand`, `never` |
| performance | System regression | `{ current: number, min: number, max: number, debounce: number, regress: () => void }` |
| size | Canvas size in pixels | `{ width: number, height: numberm top: numberm left: numberm updateStyle?: boolean }` |
| size | Canvas size in pixels | `{ width: number, height: number, top: number, left: number }` |
| viewport | Viewport size in three.js units | `{ width: number, height: number, initialDpr: number, dpr: number, factor: number, distance: number, aspect: number, getCurrentViewport: (camera?: Camera, target?: THREE.Vector3, size?: Size) => Viewport }` |
| xr | XR interface, manages WebXR rendering | `{ connect: () => void, disconnect: () => void }` |
| set | Allows you to set any state property | `(state: SetState<RootState>) => void` |
| get | Allows you to retrieve any state property non-reactively | `() => GetState<RootState>` |
| invalidate | Request a new render, given that `frameloop === 'demand'` | `() => void` |
| advance | Advance one tick, given that `frameloop === 'never'` | `(timestamp: number, runGlobalEffects?: boolean) => void` |
| setSize | Resize the canvas | `(width: number, height: number, updateStyle?: boolean, top?: number, left?: number) => void` |
| setSize | Resize the canvas | `(width: number, height: number, top?: number, left?: number) => void` |
| setDpr | Set the pixel-ratio | `(dpr: number) => void` |
| setFrameloop | Shortcut to set the current render mode | `(frameloop?: 'always', 'demand', 'never') => void` |
| setEvents | Shortcut to setting the event layer | `(events: Partial<EventManager<any>>) => void` |
Expand Down
17 changes: 4 additions & 13 deletions packages/fiber/src/core/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,22 +161,13 @@ export type ReconcilerRoot<TCanvas extends Element> = {
unmount: () => void
}

function isCanvas(maybeCanvas: unknown): maybeCanvas is HTMLCanvasElement {
return maybeCanvas instanceof HTMLCanvasElement
}

function computeInitialSize(canvas: HTMLCanvasElement | OffscreenCanvas, defaultSize?: Size): Size {
if (defaultSize) {
return defaultSize
}

if (isCanvas(canvas) && canvas.parentElement) {
function computeInitialSize(canvas: HTMLCanvasElement | OffscreenCanvas, size?: Size): Size {
if (!size && canvas instanceof HTMLCanvasElement && canvas.parentElement) {
const { width, height, top, left } = canvas.parentElement.getBoundingClientRect()

return { width, height, top, left }
}

return { width: 0, height: 0, top: 0, left: 0 }
return { width: 0, height: 0, top: 0, left: 0, ...size }
}

function createRoot<TCanvas extends Element>(canvas: TCanvas): ReconcilerRoot<TCanvas> {
Expand Down Expand Up @@ -334,7 +325,7 @@ function createRoot<TCanvas extends Element>(canvas: TCanvas): ReconcilerRoot<TC
// Check size, allow it to take on container bounds initially
const size = computeInitialSize(canvas, propsSize)
if (!is.equ(size, state.size, shallowLoose)) {
state.setSize(size.width, size.height, size.updateStyle, size.top, size.left)
state.setSize(size.width, size.height, size.top, size.left)
}
// Check frameloop
if (state.frameloop !== frameloop) state.setFrameloop(frameloop)
Expand Down
18 changes: 7 additions & 11 deletions packages/fiber/src/core/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export type Subscription = {
}

export type Dpr = number | [min: number, max: number]
export type Size = { width: number; height: number; top: number; left: number; updateStyle?: boolean }
export type Size = { width: number; height: number; top: number; left: number }
export type Viewport = Size & {
/** The initial pixel ratio */
initialDpr: number
Expand Down Expand Up @@ -148,12 +148,8 @@ export type RootState = {
advance: (timestamp: number, runGlobalEffects?: boolean) => void
/** Shortcut to setting the event layer */
setEvents: (events: Partial<EventManager<any>>) => void
/**
* Shortcut to manual sizing
*
* @todo before releasing next major version (v9), re-order arguments here to width, height, top, left, updateStyle
*/
setSize: (width: number, height: number, updateStyle?: boolean, top?: number, left?: number) => void
/** Shortcut to manual sizing */
setSize: (width: number, height: number, top?: number, left?: number) => void
/** Shortcut to manual setting the pixel ratio */
setDpr: (dpr: Dpr) => void
/** Shortcut to setting frameloop flags */
Expand Down Expand Up @@ -248,7 +244,7 @@ const createStore = (
},
},

size: { width: 0, height: 0, top: 0, left: 0, updateStyle: false },
size: { width: 0, height: 0, top: 0, left: 0 },
viewport: {
initialDpr: 0,
dpr: 0,
Expand All @@ -264,9 +260,9 @@ const createStore = (

setEvents: (events: Partial<EventManager<any>>) =>
set((state) => ({ ...state, events: { ...state.events, ...events } })),
setSize: (width: number, height: number, updateStyle?: boolean, top?: number, left?: number) => {
setSize: (width: number, height: number, top: number = 0, left: number = 0) => {
const camera = get().camera
const size = { width, height, top: top || 0, left: left || 0, updateStyle }
const size = { width, height, top, left }
set((state) => ({ size, viewport: { ...state.viewport, ...getCurrentViewport(camera, defaultTarget, size) } }))
},
setDpr: (dpr: Dpr) =>
Expand Down Expand Up @@ -370,7 +366,7 @@ const createStore = (
// Update camera & renderer
updateCamera(camera, size)
gl.setPixelRatio(viewport.dpr)
gl.setSize(size.width, size.height, size.updateStyle)
gl.setSize(size.width, size.height)
}

// Update viewport once the camera changes
Expand Down
2 changes: 1 addition & 1 deletion packages/fiber/tests/core/hooks.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('hooks', () => {
expect(result.camera instanceof THREE.Camera).toBeTruthy()
expect(result.scene instanceof THREE.Scene).toBeTruthy()
expect(result.raycaster instanceof THREE.Raycaster).toBeTruthy()
expect(result.size).toEqual({ height: 0, width: 0, top: 0, left: 0, updateStyle: false })
expect(result.size).toEqual({ height: 0, width: 0, top: 0, left: 0 })
})

it('can handle useFrame hook', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/test-renderer/src/__tests__/RTTR.hooks.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('ReactThreeTestRenderer Hooks', () => {
expect(result.camera instanceof THREE.Camera).toBeTruthy()
expect(result.scene instanceof THREE.Scene).toBeTruthy()
expect(result.raycaster instanceof THREE.Raycaster).toBeTruthy()
expect(result.size).toEqual({ height: 0, width: 0, top: 0, left: 0, updateStyle: false })
expect(result.size).toEqual({ height: 0, width: 0, top: 0, left: 0 })
})

it('can handle useLoader hook', async () => {
Expand Down