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

Hotfix - Administración - Altura de IFrame no se recogía al cambiarla #502

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

ainaraRT
Copy link
Collaborator

@ainaraRT ainaraRT commented Dec 3, 2024

Description

Como se describe en el issue, no es posible establecer la altura de un campo iframe. Aunque se configure en el estudio (tampoco se cambia por el valor nuevo), al insertarlo en una vista de detalle de cualquier módulo, la altura no se respeta. Incluso, creando un campo en el Constructor de Módulos.
Esto es debido a que en Estudio, al cambiar la altura, no se respeta el nuevo valor que se le establecía, sino que volvía a guardarse con el valor original.
Como tal, el rango de la altura es de 100 a 1024, por lo que menor o mayor a este rango no se cambiará.

Motivation and Context

Se ha modificado el código establecido en el TemplateIframe.php que hace que se muestre en la vista de Detalle (get_xtpl_detail()).
Además de hacer que se guarde en el fichero vardef correspondiente o estableciendo la altura predeterminada:
$to_save['link_target'] = (isset($field->link_target) && is_numeric($field->link_target)) ? $field->link_target : '200';

How To Test This

  1. Crear un campo, en cualquier módulo, de tipo iframe añadiendo una altura inicial de entre 100 y 1024.
  2. Agregar dicho campo a la vista de detalle.
  3. Añadir un iframe con dicha altura (ej: https://placehold.co/300), volver a cargar la página y comprobar que es la atura definida.
  4. Volver a estudio y editar la altura.
  5. Comprobar que se ha guardado correctamente, dentro de estudio, y comprobarlo en la vista de detalle

Adicional

  • Hacer las pruebas con el Constructor de Módulos

@ainaraRT ainaraRT self-assigned this Dec 3, 2024
Copy link

github-actions bot commented Dec 3, 2024

Actions executed at: 2025-01-24 13:24:42.

Copy link
Collaborator

@PaulaaSTIC PaulaaSTIC left a comment

Choose a reason for hiding this comment

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

No se recoge bien la altura del iframe cuando es 100, no se guarda en estudio.

@ainaraRT ainaraRT requested a review from PaulaaSTIC January 8, 2025 10:15
PaulaaSTIC
PaulaaSTIC previously approved these changes Jan 13, 2025
Copy link
Collaborator

@PaulaaSTIC PaulaaSTIC left a comment

Choose a reason for hiding this comment

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

(A)probado, se modificia el tamaño del iframe conforme el valor establecido en estudio.

Copy link
Collaborator

@juanSTIC juanSTIC left a comment

Choose a reason for hiding this comment

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

En mi opinión el plateamiento utilizado es icorrecto. Se está actualizando el valor de ext4 en fields_meta_data, lo cual es algo no habitual en el CRM, además de nos ser necesario. Cuando en estudio se modifica la altura del iframe, esta queda correcatmente refljada en el fichero vardef correspondiente, por ejemplo:

<?php
// custom/Extension/modules/stic_Sepe_Actions/Ext/Vardefs/_override_sugarfield_test_c.php
// ...
$dictionary['stic_Sepe_Actions']['fields']['test_c']['link_target']='110';
// ...

Los valores de fields_meta_data quedan exclusivamente para la el momento de la creación del campo.

La solución del PR debería de hacerse en el punto dónde se recuperan los valores del bean para renderizar el iframe y no dónde se hacen ahora.

Copy link
Collaborator

@juanSTIC juanSTIC left a comment

Choose a reason for hiding this comment

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

En mi opinión el plateamiento utilizado es icorrecto. Se está actualizando el valor de ext4 en fields_meta_data, lo cual es algo no habitual en el CRM, además de nos ser necesario. Cuando en estudio se modifica la altura del iframe, esta queda correcatmente refljada en el fichero vardef correspondiente, por ejemplo:

<?php
// custom/Extension/modules/stic_Sepe_Actions/Ext/Vardefs/_override_sugarfield_test_c.php
// ...
$dictionary['stic_Sepe_Actions']['fields']['test_c']['link_target']='110';
// ...

La solución del PR debería contemplar la lectura/escritura del nuevo valor de altura del iframe en el fichero vardef correspondiente, no en fields_meta_data cuyos valores quedan exclusivamente para la el momento de la creación del campo.

Una de las consecuencias del enfoque actual es que la solución solamente sirve para los campos creados en estudio, pero no para los hechos con el constructor de módulos, ya que estos no utilizan la tabla fields_meta_data

Copy link
Collaborator

@juanSTIC juanSTIC left a comment

Choose a reason for hiding this comment

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

Veo los siguientes errores:

  • Al crear un nuevo campo iframe en estudio (antes de modificarlo nuevamente) no se utiliza la altura definida en estudio, siempre se renderiza con un 200px independientemente de lo indicado al crear el campo.

  • Al modificar el campo en estudio, se escribe en el fichero vardef las propiedades del campo del siguiente modo:

       $dictionary['stic_Sepe_Actions']['fields']['test_iframe_c']['link_target']='155';
       $dictionary['stic_Sepe_Actions']['fields']['test_iframe_c']['height']='125';
    
    • La primera de las propiedades (link_target) es errónea, ya que link_target nada tiene que ver con la altura y no debería usarse en este PR, sin embargo el valor de esta propiedad es el que se cargará al modificar el campo en estudio.
    • La segunda de las propiedades (height) es la que se debería estar usando en cualquiera de los casos, ya que es la que se utiliza para renderizar el campo en las vistas y la que debería usarse para recuperar el valor del campo en estudio.

@ainaraRT ainaraRT requested a review from juanSTIC January 20, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incidencia - General - No se respeta la altura definida en campo iframe en vistas de detalle
3 participants