MecatronicaUncu / Red-Social-Asociacion

A small open source social network for any small community
GNU General Public License v2.0
3 stars 1 forks source link

Feature/119 multiples actividades #176

Closed francoa closed 7 years ago

francoa commented 8 years ago

Estoy de vuelta muchachos. Y LES TRAIGO AMOR.

Además de amor traigo quilombo, para variar:

  1. No me puteen mucho por las tabulaciones por favor.
  2. No mergeen esto aún. Hago el PR para charlar un par de cositas.

Bien, lo que hice fue usar la magia de jQuery. Volé el ng-repeat del html (es muy bonita la forma angular, pero creo que en este caso no se justifica, después de todo siempre son 7 días por semana) para poder agregar y sacar filas de la tabla sin tener que pelear con los tiempos que se toma angular para actualizar la tabla.

Puedo hacerlo con menos código, pero ya para que vayan probando. Agreguen todas los horarios que quieran que se superpongan (el límite son las estrellas). Si falla es porque no lo probé lo suficiente jaja.

Por otra parte, creo que el código del archivo edt.js (frontend, en general) puede ordenarse y optimizarse un poco. Creo que en algún momento habíamos hablado de esto, no?

francoa commented 8 years ago

A y por cierto @andresmanelli no supe como hacer andar las translations desde el código js

andresmanelli commented 8 years ago

Buenisimo. Great to have you back. Ya me voy a poner a probar esto.

Por el tema de la carga de Angular y lo de las traducciones: Para que todo ande bien habría que hacer (me parece) de cada fila una directive, y de cada bloque de horario otra directive que se vayan cargando dentro del html de la fila. Y así como estás cargando html con jQuery, cargar el html de la directive (así angular dsp puede parsear y hacer los bindings). Encontré esto así rápido, no sé si vienen por ahí la mano: LINK LINK

De todas maneras eso puede quedar como issue para otro momento.

francoa commented 8 years ago

Si igual yo pensaba en una forma más sencilla, para no agregar más ruido del que ya hay. Ahí lo encontré, hago commit. Era más simple de lo que pensaba

francoa commented 8 years ago

Mm, bueno, el único tema es que cuando tocás F5 explota porque dice que no encuentra las translations en la session

fcladera commented 8 years ago

Si podés rebaseá a develop, hay varias cosas que han cambiado en ambos commits (pero el otro entró antes antes :smiley:)

francoa commented 8 years ago

Pulgar arriba

andresmanelli commented 8 years ago

Anda muy bien pero no sé por qué me flashó esto:

asd

voy a seguir probando.

andresmanelli commented 8 years ago

Debe haber sido ruido en la BDD. No lo pude reproducir, asique hagan de cuenta que no dije nada

andresmanelli commented 8 years ago

Lo que sí estaría bueno agregar (para otro issue) es un css para que haga la línea de separación los días un poquito más gruesa para que se vea mejor cuáles corresponden al mismo y cuáles no.

Muy bueno. Yo voto merge.

francoa commented 8 years ago

Hay q verlo porque a mi tmbn me pasó algo parecido una vez cuando levanté la consola de desarrollo y nunca lo pude reproducir jajaj

andresmanelli commented 8 years ago

Encontré esto que es muy bueno:

Add ?w=1 to the URL to see the diff with whitespace ignored.

andresmanelli commented 8 years ago

Loco, pongo un par de comments que hice del código para ver cómo andaba. Podés agregarlos ? Cambiando lo que haya que cambiar.

Hay algunos que son más de doc y otros son preguntar para mejorar detalles.

diff --git a/src/app/edt/edt.js b/src/app/edt/edt.js
index d90cdc4..e0f8ca6 100755
--- a/src/app/edt/edt.js
+++ b/src/app/edt/edt.js
@@ -197,6 +197,7 @@
       };
       /** @type {Array} Guarda las <div>s que muestran las franjas horarias para poder eliminarlas */
       $scope.divs = [];
+      // Unificar las dos?
       /** @type {Array} Guarda las <tr>s que muestran las franjas horarias para poder eliminarlas */
       $scope.trs = [];
       /** @type {Array} Guarda las <tr>s que muestran las franjas horarias para poder eliminarlas */
@@ -501,34 +502,48 @@

           if ($scope.suffix == 'H') {

+            // Offset from start
             var x = ((el.mti - start) / tt) * divwidth;
+            // Width of element
             var w = ((el.mtf - el.mti) / tt) * divwidth;

             // Superposition verification. If after this loop, tdId is null, then a new row is needed
+            // tdIs es el id de la primera div en donde podemos colocar el elemento actual
             var tdId = null;
+            // Es el index de la tr que contiene al elemento actual! Se busca en $scope.trs.forEach(...)
             var trIndex = 0;
             if (localIndex > 0){
               id = null;
               var name = $scope.daysToShow[el.day - 1].name;
+              // Hace el bucle para encontrar la buena tr? mejor hacer .some(), poque trs siempre tiene 7 elementos
               $scope.trs.forEach(function(tr,index){
                 if (tr.indexOf(name)>=0){
                   trIndex = index;
+                  // tdId always null as parameter?
                   tdId = funcionTr(tr,tdId,x,w);
                 }
               });
               if (tdId == null){
+                // Verificamos la superposición en las tr adicionales para este día
+                // Este bucle nos está devolviendo la última tr que puede almacenarlo, no la primera
+                // si se quiere la primera conviene usar some()
+                // Hacer de addTrs un objeto para poder hacer addTrs[name].forEach() ?
                 $scope.additionalTrs.forEach(function(tr,index){
                   if (tr.indexOf(name)>=0){
-                    trIndex = index;//?
+                    trIndex = index;//? No seria valido porque se usa con el array trs, no con addTrs
+                    // tdId always null as parameter?
                     tdId = funcionTr(tr,tdId,x,w);
                   }
                 });
               }
             }

+            // Encontramos en qué tr ubicar el elemento. La div dentro de la tr que contiene los elementos es 
+            // $("#"+tdId).children()[0]
             if (tdId != null){
               id = $("#"+tdId).children()[0].id;
             }
+            // Unificar tdId con id ?
             else if (id == null){
               // New row needed
               id = $scope.daysToShow[el.day - 1].name + 'T' + $scope.additionalTrs.length;
@@ -589,7 +604,12 @@
       var funcionTr = function(tr,tdId,x,w){
         var td = tr.replace("tr","td");
         var superposition = false;
+        // $("#"+td).children()[0] es una de las divs que contiene los bloques de horarios para este día
+        // var divTimes = ...
+        // divTimes.children() son los bloques de horarios de esa div y tr
+        // guardar en variable divTimes.children para mejor lectura?
         $("#"+$("#"+td).children()[0].id).children().each(function(divIndex){
+          // Esta div es un bloque de horario
           var div = $("#"+$("#"+td).children()[0].id).children()[divIndex];
           var oldX = $("#"+div.id)[0].offsetLeft;
           var oldW = $("#"+div.id)[0].offsetWidth;
@@ -599,8 +619,9 @@
           }
         });
         if (superposition === false){
+          // La div donde se va a guardar el elemento actual es la misma que se verifica, porque no hay superposición
           tdId = td;
-        }
+        } //else la div es null y se debe generar otra (o verificar otra tr si ya se han agregado)
         return tdId;
       };

Dejo el archivo para que sea fácil hacer patch si lo querés usar: comments.txt

fcladera commented 8 years ago

Acabo de pushear un hotfix por el tema de la img de Docker, si podés @francoa hacé un rebase a develop.