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

Ejercicios de reactive-forms #44

Open
wants to merge 1 commit into
base: reactive-forms
Choose a base branch
from

Conversation

Gabriel-Campitelli
Copy link

Hola profe! Acá subo mi resolución a los ejercicios de reactive-forms.

Saludos.

export class LocalStorageService {
// https://developer.mozilla.org/es/docs/Web/API/Window/localStorage
constructor() { }
getName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

para que es esta clase?

Copy link
Author

Choose a reason for hiding this comment

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

esto me lo debí haber traído cuando hice el clone del repo porque no lo escribí yo; de todas formas lo tendría que haber revisado y eliminado

private service: TodoService
) { }

ngOnInit(): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

sino hace nada no hace falta el metodo

@@ -0,0 +1,2 @@
<h2>Tasks Completed {{completedSize()}}</h2>
<h2>Tasks To do {{incompletedSize()}} </h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

los archivos vacios de abajo habria que eliminarlos

constructor(){

this.taskForm = new FormGroup({
newTaskControl: new FormControl('',[Validators.required]),
Copy link
Contributor

Choose a reason for hiding this comment

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

tal vez convendria nombrar los controles directamente con lo que tienen como name y url, en estos casos creo que menos es mas.

Copy link
Contributor

Choose a reason for hiding this comment

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

quiero decir, hay que ser descriptivos, pero por ahi no es necesario aclarar que tipo es la variable, eso surge del propio sistema de tipos del lenguaje

Copy link
Author

Choose a reason for hiding this comment

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

Buenisimo profe!! lo corrijo

this.add.emit(task);

this.taskForm.reset();
document.getElementById("button").textContent = "Create";
Copy link
Contributor

Choose a reason for hiding this comment

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

esto no es correcto en angular, si tenes que modificar el dom, es mejor usar una variable (por ej. buttonLabel )que tenga el string que queres reemplazar y asignarlo aca, y en el template pones {{buttonLabel}} y angular se encarga de modificar el DOM. Solo en casos puntuales es bueno modificar el DOM, si angular te permite hacerlo mejor hacelo directo con angular.

Copy link
Contributor

@aotaduy aotaduy left a comment

Choose a reason for hiding this comment

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

Habria que eliminar las referencias al DOM, por lo demas se ve bien


ngOnChanges(){
if(this.item != null){
document.getElementById("button").textContent = "Edit";
Copy link
Contributor

Choose a reason for hiding this comment

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

mismo de arriba

}

getName() {
return 'TodoService 123' + this.storage.getName();
Copy link
Contributor

Choose a reason for hiding this comment

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

para que es esto ?

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.

2 participants