UDA-EJIE / udaRUP

RUP components repository
Other
13 stars 10 forks source link

[rup_table] Eliminar requisito de ID 'xxxYYYWar_content' innecesario #166

Closed ihernaez closed 4 years ago

ihernaez commented 4 years ago

En la documento de actualización a la versión 4.2.1 se indica la necesidad de que el <div> con el contenido de la aplicación tenga un ID con la forma xxxYYYWar_content.

Revisando los cambios de la versión vemos que el motivo para dicho requisito es la creación de un elemento <a> temporal para la apertura del contenido de los informes:

$("div#" + $.rup.WAR_NAME + "War_content").append("<a id='rupTableButtonsReportsExport' class='d-none'>rupTableButtonsReportsExport</a>");

Este requisito es innecesariamente restrictivo e inflexible. El enlace se puede crear perfectamente en los elementos relativos al componente. Por ejemplo:

$('#' + ctx.sTableId + 'rup_report_dialogsContainer').append("<a id='rupTableButtonsReportsExport' class='d-none'>rupTableButtonsReportsExport</a>");
xaabi6 commented 4 years ago

Hola @ihernaez, originalmente este cambio se hizo para poder diferenciar mejor el elemento div que alberga el contenido, ya que hasta ahora, se hacía a través de la clase .content, siendo el nombre de esta clase susceptible a ser usado en otro punto de la aplicación y pudiendo así causar errores.

Tras la realización de este cambio, se decidió crear el elemento a temporal usando el identificador del elemento div que alberga el contenido como padre, asegurándonos así que encontrásemos sólo el elemento necesario y ninguno más. En la versión 4.2.0 de UDA, se hacía así:

$("div.content").append("<a id='rupTableButtonsReportsExport' class='d-none'>rupTableButtonsReportsExport</a>");

Respondiendo ahora a tu sugerencia, sí, podría haberse hecho tal y como propones, ¿pero qué problema hay en que sea restrictivo?, al fin y al cabo, es un elemento temporal que necesita UDA para la descarga del documento generado y que no debería de mostrarse nunca al usuario ni ser usado para otros fines.

ihernaez commented 4 years ago

Hola @xaabi6 ,

Nuestro problema particular es que las plantillas con la estructura general y los componentes comunes son compartidas entre las aplicaciones a través de una librería. Al adaptar estas plantillas generales a los cambios de UDA 4.2.1, y al ser una versión de "patch", es cuando se investigó la causa de semejante cambio. En esta librería se utilizaban otros ID independientes de la aplicación para los componentes comunes, cuyas referencias habrá que cambiar a nivel global, pero se puede afrontar (preferiblemente eliminando las referencias por ID actuales).

La sugerencia se debe más a una cuestión conceptual: Requerir un elemento externo al ámbito del componente con un ID fijo y no configurable (véase la opción appendTo de jQuery.Dialog) es un "smell" de diseño claro.

Excepto para casos en los que la posición de los elementos en el DOM vayan a afectar como se visualicen (Dialog otra vez) los elementos deberían, en mi opinión, crearse relativos al elemento de la instancia del componente y para elementos que se esperan únicos, como es el caso, utilizarse los creados por el propio Widget (De ahí la sugerencia de usar rup_report_dialogsContainer, aunque la referencia por ID tampoco me convence por las potenciales colisiones de ID que mencionas).

En cualquier caso la sugerencia es evitar los requisitos esctrictos en los componentes excepto cuando sea absolutamente necesario.

Pero es eso: una sugerencia :stuck_out_tongue_winking_eye:

xaabi6 commented 4 years ago

Hola @ihernaez, finalmente hemos adoptado el cambio que nos propusiste inicialmente. Consideramos que es buena idea que el enlace temporal sea relativo a los elementos del dom generados por el componente.

sergisu commented 4 years ago

Dejamos el issue abierto hasta consolidar el cambio en la release.