Skip to content

Fix/dropdown input result callbacks #1071

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.

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

Merged
merged 7 commits into from
Feb 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 25 additions & 31 deletions packages/@dcl/react-ecs/src/reconciler/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { Entity, IEngine, InputAction, PointerEventsSystem, PointerEventType } from '@dcl/ecs'
import {
Entity,
IEngine,
InputAction,
PointerEventsSystem,
PointerEventType,
PBUiInputResult,
PBUiDropdownResult
} from '@dcl/ecs'
import * as components from '@dcl/ecs/dist/components'
import Reconciler, { HostConfig } from 'react-reconciler'
import { Callback, isListener, Listeners } from '../components'
Expand Down Expand Up @@ -183,6 +191,7 @@ export function createReconciler(

function removeChildEntity(instance: Instance) {
changeEvents.delete(instance.entity)
clickEvents.delete(instance.entity)
engine.removeEntity(instance.entity)
for (const child of instance._child) {
removeChildEntity(child)
Expand Down Expand Up @@ -233,13 +242,24 @@ export function createReconciler(
}

function updateOnChange(entity: Entity, componentId: number, state?: OnChangeState) {
const hasEvent = changeEvents.has(entity)
const event = changeEvents.get(entity) || changeEvents.set(entity, new Map()).get(entity)!
const oldState = event.get(componentId)
const onChangeCallback = state?.onChangeCallback
const onSubmitCallback = state?.onSubmitCallback
const value = state?.value ?? oldState?.value
const isSubmit = state?.isSubmit ?? oldState?.isSubmit
event.set(componentId, { onChangeCallback, onSubmitCallback, value, isSubmit })
event.set(componentId, { onChangeCallback, onSubmitCallback })
// Create onChange callback if its the first callback event for this entity
if (!hasEvent) {
const resultComponentId =
componentId === UiDropdown.componentId ? UiDropdownResult.componentId : UiInputResult.componentId
engine.getComponent<PBUiInputResult | PBUiDropdownResult>(resultComponentId).onChange(entity, (value) => {
if ((value as PBUiInputResult)?.isSubmit) {
const onSubmit = changeEvents.get(entity)?.get(componentId)?.onSubmitCallback
onSubmit && onSubmit(value?.value)
}
const onChange = changeEvents.get(entity)?.get(componentId)?.onChangeCallback
onChange && onChange(value?.value)
})
}
}

const hostConfig: HostConfig<
Expand Down Expand Up @@ -352,34 +372,8 @@ export function createReconciler(
null
)

// Maybe this could be something similar to Input system, but since we
// are going to use this only here, i prefer to scope it here.
function handleOnChange(componentId: number, resultComponent: typeof UiDropdownResult | typeof UiInputResult) {
for (const [entity, Result] of engine.getEntitiesWith(resultComponent)) {
const entityState = changeEvents.get(entity)?.get(componentId)
const isSubmit = !!(Result as any).isSubmit
if (entityState?.onChangeCallback && Result.value !== entityState.value) {
entityState.onChangeCallback(Result.value)
}
if (entityState?.onSubmitCallback && isSubmit && !entityState.isSubmit) {
entityState.onSubmitCallback(Result.value)
}

updateOnChange(entity, componentId, {
onChangeCallback: entityState?.onChangeCallback,
onSubmitCallback: entityState?.onSubmitCallback,
value: Result.value,
isSubmit
})
}
}

return {
update: function (component: ReactEcs.JSX.ReactNode) {
if (changeEvents.size) {
handleOnChange(UiInput.componentId, UiInputResult)
handleOnChange(UiDropdown.componentId, UiDropdownResult)
}
return reconciler.updateContainer(component as any, root, null)
},
getEntities: () => Array.from(entities)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export function createRendererTransport(engineApi: EngineApiForTransport): Trans
) {
return false
}

return !!message
},
type: 'renderer'
Expand Down
4 changes: 3 additions & 1 deletion test/react-ecs/dropdown.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ describe('UiDropdown React ECS', () => {
})

it('remove the onchange fn, so if we change the value there is no call to the fn because there is no onChange fn', async () => {
expect(onChange).toBeCalledTimes(0)
onChange.mockClear()
conditional = false
expect(onChange).toBeCalledTimes(0)
await engine.update(1)

UiDropdownResult.getMutable(uiEntity).value = 2
Expand All @@ -78,6 +79,7 @@ describe('UiDropdown React ECS', () => {
})

it('put the conditional en true, so we have the onChange fn again', async () => {
onChange.mockClear()
conditional = true
await engine.update(1)
expect(onChange).toBeCalledTimes(0)
Expand Down
1 change: 1 addition & 0 deletions test/react-ecs/input.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ describe('Ui Listeners React Ecs', () => {
UiInputResult.getMutable(uiEntity).isSubmit = true
await engine.update(1)
expect(onSubmit).toBeCalledTimes(1)
onChange.mockClear()
removeComponent = true
await engine.update(1)
UiInputResult.create(uiEntity, { value: 'BOEDO' })
Expand Down
12 changes: 6 additions & 6 deletions test/snapshots/production-bundles/ui.ts.crdt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
SCENE_COMPILED_JS_SIZE_PROD=353.8k bytes
SCENE_COMPILED_JS_SIZE_PROD=353.6k bytes
(start empty vm 0.21.0-3680274614.commit-1808aa1)
OPCODES ~= 0k
MALLOC_COUNT = 1005
Expand All @@ -9,7 +9,7 @@ EVAL test/snapshots/production-bundles/ui.js
REQUIRE: ~system/EngineApi
REQUIRE: ~system/EngineApi
OPCODES ~= 65k
MALLOC_COUNT = 19507
MALLOC_COUNT = 19502
ALIVE_OBJS_DELTA ~= 3.88k
CALL onStart()
OPCODES ~= 0k
Expand Down Expand Up @@ -48,11 +48,11 @@ CALL onUpdate(0)
Scene: PUT_COMPONENT e=0x209 c=1093 t=1 data={"placeholder":"","disabled":false}
Scene: PUT_COMPONENT e=0x201 c=1094 t=1 data={"acceptEmpty":false,"options":["BOEDO","CASLA"],"selectedIndex":0,"disabled":false,"color":{"r":1,"g":0,"b":0,"a":1}}
OPCODES ~= 134k
MALLOC_COUNT = 678
ALIVE_OBJS_DELTA ~= 0.24k
MALLOC_COUNT = 705
ALIVE_OBJS_DELTA ~= 0.25k
CALL onUpdate(0.1)
Scene: PUT_COMPONENT e=0x200 c=1 t=2 data={"position":{"x":8,"y":1,"z":8},"rotation":{"x":0,"y":0.008726535364985466,"z":0,"w":0.9999619126319885},"scale":{"x":1,"y":1,"z":1},"parent":0}
OPCODES ~= 66k
OPCODES ~= 65k
MALLOC_COUNT = 250
ALIVE_OBJS_DELTA ~= 0.12k
CALL onUpdate(0.1)
Expand All @@ -65,4 +65,4 @@ CALL onUpdate(0.1)
OPCODES ~= 64k
MALLOC_COUNT = 0
ALIVE_OBJS_DELTA ~= 0.00k
MEMORY_USAGE_COUNT ~= 1695.75k bytes
MEMORY_USAGE_COUNT ~= 1695.42k bytes
Loading