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

ALTO ANGULAR PAPA #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ALTO ANGULAR PAPA #23

wants to merge 2 commits into from

Conversation

GustavoBilzBorre
Copy link
Collaborator

Separé el botón de aceptar de la parte de NUEVO de manera cabeza. Si alguien lo quiere poner de forma prolija, díganme cómo.

@leamarty
Copy link
Owner

leamarty commented Jan 8, 2016

No veo foto. El PR tiene que ir acompañado de uno o dos screenshots.

<tbody>
<tr ng-repeat="cliente in clientes">
<td>{{ cliente.id }}</td>
<td>{{ cliente.razon_social }}</td>
<td ng-repeat="(key, value) in cliente">{{ value }}</td>
Copy link
Owner

Choose a reason for hiding this comment

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

Qué pasa si del backend te llegan en otro orden los atributos del cliente?

Entiendo la idea de hacer todo super genérico y con un loop, pero acá no hace falta ni tampoco mejora las cosas.

THs estáticos, y TD's "estáticos". Que sólo se iteren los TRs

Copy link
Owner

Choose a reason for hiding this comment

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

Y el botón de eliminar?

@leamarty leamarty assigned dragonnegro78 and unassigned leamarty Jan 8, 2016
@leamarty
Copy link
Owner

leamarty commented Jan 8, 2016

@dragonnegro78 pegale una mirada porfa, y después devolvéselo a Bimbo

$scope.headers = [
'ID',
'Razon Social',
'Saldo Positivo',
Copy link
Collaborator

Choose a reason for hiding this comment

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

¿Que es esto de saldo positivo y saldo negativo?
En la tabla del listado de clientes tiene que haber una unica columna "Patrimonio neto" que sea la diferencia entre el activo y el pasivo (salvo positivo y negativo respectivamente en tu caso).
O sea, no esta mal que en $scope.clientes tengas ambos, pero si esta mal que se muestren ambos al usuario.
Si no flasheo, podes hacer <td>{{ cliente.saldo_positivo - cliente.salgo_negativo }}</td> en html/cliente/listado.html

@dragonnegro78
Copy link
Collaborator

No se puede mergear a master bimbo.
Si la practica-modulo5 no estaba creada cuando empezaste a escribir el codigo, tendrias que haberla hecho vos y luego brancheado desde esa.

@dragonnegro78
Copy link
Collaborator

¿Quien es Fernando Arias?
Github solo te deja a vos hacer los pull request, no a gente de afuera, pero ellos si pueden ser los autores de un commit.
¿Tenes mal configurado git porque estuviste trabajando con otra cuenta y te olvidaste de cambiarlo?
Fijate que hay una opcion para establecer el usuario por proyecto, no de forma global en toda la compu. Tenes que pararte en cursoweb/m5 desde la consola y tirar comandos ahi, pero no recuerdo de memoria cuales son (para cambiar el usuario, etc.)

<tbody>
<tr ng-repeat="cliente in clientes">
<td>{{ cliente.id }}</td>
<td>{{ cliente.razon_social }}</td>
<td>{{ cliente.id }}</td><td>{{ cliente.razon_social }}</td><td>{{ cliente.saldo_positivo }}</td><td>{{ cliente.saldo_negativo }}</td><td><button class="btn btn-danger">X</button></td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Intenta poner estas cosas en multiples lineas porque facilita la lectura, de vos y del resto.
No se si sera un bug del editor de texto que usas pero con phpstorm casi siempre te empieza cosas en la linea siguiente. ¿Lo desconfiguraste?

Copy link
Owner

Choose a reason for hiding this comment

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

Sí por favor, múltiples lineas.

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.

3 participants