Skip to content

New tag component#100

Open
GonzzaG wants to merge 3 commits into
devfrom
NewTagComponent
Open

New tag component#100
GonzzaG wants to merge 3 commits into
devfrom
NewTagComponent

Conversation

@GonzzaG

@GonzzaG GonzzaG commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

})
}

updateBiomarkerTag = async (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dos correcciones sobre esta función:

  1. Documentala con JSDocs como hacemos con todas las demás.
  2. Pasalo a promises en vez de async/try/catch. Pedile a cualquier LLM que te haga la conversión. Pero estamos haciendo así en todo el proyecto, prefiero que no quede mixto. Después voy a ver si puedo agregar una regla de ESLint para que advierta sobre esto

.finally(() => setLoadingTags(false))
}

// Fetches all the Tags as soon as the component is mounted

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Poner como JSDocs (es decir, con la sintaxis /** ... */). De esa manera los IDEs pueden renderizarla cuando se le hace hover con el mouse

setNewTag(getDefaultNewTag())
}

// Keeps the internal selection in sync if the parent updates the Tag from outside

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem comentario anterior sobre JSDocs

}

// Keeps the internal selection in sync if the parent updates the Tag from outside
// while the panel is closed (e.g. after a refresh of the underlying data)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem comentario anterior sobre JSDocs

handleClose()
}

// Discards any pending change made during this opening of the Dropdown

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem comentario anterior sobre JSDocs

* @param tag Tag to delete.
*/
const deleteTag = (e: React.MouseEvent<HTMLElement>, tag: DjangoTag) => {
e.preventDefault()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agregar un mini comentario de por qué estamos haciendo esta línea y la de abajo. Si da lo mismo que estén, borrar, si son importantes explicar por qué

})
}

// Sends the request to create or update a Tag and selects it as soon as it's saved

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Idem comentario anterior sobre JSDocs

/**
* Fetches the User's defined Biomarker/Experiment Tags.
*/
getUserTags () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Esto ya lo vi en otro archivo, esta función debería moverse dentro de TagDropdown.tsx, el TagType podría ser una prop del componente. Y el listado de los Tags disponibles debería estar en el estado de TagDropdown (así puede hacerse ahí dentro también el const tagOptions: DropdownItemProps[]....

En caso de que por algún motivo raro no se pueda (preguntame por fa!), extraer esta función a utils.ts o algún archivo de utilitarios. Que reciba por parámetro el TagType y usar esa función en común en todos los lugares donde se necesite.

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