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.

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

controlar re-renderizado en pantalla desafío #300

Merged
merged 5 commits into from
Jun 28, 2024

Conversation

danielferro69
Copy link
Contributor

Resolves #298

vokoscreenNG-2024-06-18_11-42-06.mp4
vokoscreenNG-2024-06-18_11-42-46.mp4

@danielferro69 danielferro69 requested a review from a team as a code owner June 18, 2024 14:43
Comment on lines 82 to 87
const vBlocklyArea = useMemo<JSX.Element>( () => {
return <VerticalChallengeWorkspace blocklyWorkspaceProps={blocklyWorkspaceProps} challenge={challenge} />}, [] );

const hBlocklyArea = useMemo<JSX.Element>( () => {
return <HorizontalChallengeWorkspace blocklyWorkspaceProps={blocklyWorkspaceProps} challenge={challenge} />}, [] );

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¿Son necesarios estos useMemo? Pregunto porque en cada componente se está haciendo tambien un useMemo.

Se podría simplificar con algo como:

  const challengeWorkspace = useMemo<JSX.Element>(
    () => { return isSmallScreen ? <VerticalChallengeWorkspace blocklyWorkspaceProps={blocklyWorkspaceProps} challenge={challenge} /> : <HorizontalChallengeWorkspace blocklyWorkspaceProps={blocklyWorkspaceProps} challenge={challenge} /> }, []
  )

Para que quede un único useMemo.

return <>
<Stack flexGrow={1} direction='column' height='100%'>
<StatementDescription
text={descriptionOrClue}
setShowStatement={setToShow}
clueIsEnabled={clue !== ''}
urlImage={challenge.imageURL()} />
{isSmallScreen ? <VerticalChallengeWorkspace blocklyWorkspaceProps={blocklyWorkspaceProps} challenge={challenge} /> : <HorizontalChallengeWorkspace blocklyWorkspaceProps={blocklyWorkspaceProps} challenge={challenge} />}
{isSmallScreen ? vBlocklyArea : hBlocklyArea }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Y acá usar el challengeWorkspace de arriba directamente

Comment on lines 112 to 115
const blocklyWorkspace = useMemo<JSX.Element>( () => {
return <EditableBlocklyWorkspace blockIds={blocklyWorkspaceProps.blockIds} categorized={blocklyWorkspaceProps.categorized} isVertical={false} />
},[])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Y acá no hacer estos useMemo, ya que se haría arriba en el challengeWorkspace

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlopezalvas contesto sobre el último, pero da igual en cualquiera ya que es un todo.
Intenté aplicar tus sugerencias, pero al hacerlo, ya sea en el cambio de modo (de v a h) da un error (entiendo q por la consulta de SmallScreen dentro del useMemo que quiza por el tipo de componente no se pueda resolver simplemente) y en el caso de quitar estos ultimos de cada "EditableBlocklyWorkspace" deja de funcionar, por ejemplo, el uso del boton del footer de info, porque al pulsarlo, limpia el workspace de blockly, no asi las pistas...
Las pruebas sin esos memos y con los cambios propuestos, aunque coherentes, no hacen lo esperado o rompen.

@danielferro69 danielferro69 merged commit 82e07aa into develop Jun 28, 2024
10 checks passed
@danielferro69 danielferro69 deleted the controlledRerender branch June 28, 2024 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Controlar re renderizado en pantalla desafío
2 participants