caverav / auditforge

AuditForge is a pentest reporting application making it simple and easy to write your findings and generate a customizable report.
https://auditforge.feriadesoftware.cl
MIT License
20 stars 0 forks source link

UI improvements #41

Closed jllanosg closed 2 months ago

jllanosg commented 2 months ago

Nuevo

DropdownButton

Componente de botón desplegable, que muestra una serie de opciones clickeables.

Cambios

DefaultRadioGroup

SelectDropdown

SimpleInput

TextArea

UITable

TextArea

Explicación Props required

requiredField: agrega un asterisco rojo al label/title/placeholder (dependiendo de si existe el label) y agrega el prop required / aria-required al formulario, lo cual es beneficioso para la accesibilidad de la aplicación. Este prop debe ser asignado estáticamente, no con un useState.

requiredAlert: genera una alerta visual agregando un contorno rojo y un ícono de signo de exclamación en el formulario, debe ser activado cuando un usuario intenta hacer submit de un form sin completar el campo que es requerido.

iTzGooDLife commented 2 months ago

Con los cambios se quitó el padding en TextArea, veo que da más flexibilidad en la implementación, pero ojo con los que lo implementaron, ya que ahora estará diferente.

Ejemplo antes

image

Ejemplo ahora

image

Observación

Una observación, es que ahora obligatoriamente tiene fondo oscuro, al igual que otros componentes, en dicho caso, tambien se debería modificar el del RichText para no generar una "discordancia" con los estilos, adjunto ejemplo con los nuevos cambios:

image

Pegita pal UI/UX

jllanosg commented 2 months ago

Con los cambios se quitó el padding en TextArea, veo que da más flexibilidad en la implementación, pero ojo con los que lo implementaron, ya que ahora estará diferente.

Ejemplo antes

image

Ejemplo ahora

image

Observación

Una observación, es que ahora obligatoriamente tiene fondo oscuro, al igual que otros componentes, en dicho caso, tambien se debería modificar el del RichText para no generar una "discordancia" con los estilos, adjunto ejemplo con los nuevos cambios:

image

Pegita pal UI/UX

El padding no se debería utilizar en la parte más externa de componentes de UI

iTzGooDLife commented 2 months ago

Observación

Con la nueva implementación el required se manejará como prop, pero puede afectar a la visual que desde que se renderiza aparezca la alerta con rojo y demás:

image

Por lo que durante las implementaciones recomiendo utilizar un estado cuyo valor inicial sea false y se vuelva true al gatillar un evento o similar

jllanosg commented 2 months ago

Observación

Con la nueva implementación el required se manejará como prop, pero puede afectar a la visual que desde que se renderiza aparezca la alerta con rojo y demás:

image

Por lo que durante las implementaciones recomiendo utilizar un estado cuyo valor inicial sea false y se vuelva true al gatillar un evento o similar

Esto se dejó así ya que para hacer lo que tú dices se requeriría un prop extra

jllanosg commented 2 months ago

Observación

Con la nueva implementación el required se manejará como prop, pero puede afectar a la visual que desde que se renderiza aparezca la alerta con rojo y demás:

image

Por lo que durante las implementaciones recomiendo utilizar un estado cuyo valor inicial sea false y se vuelva true al gatillar un evento o similar

No recomiendo en absoluto hacer lo que mencionas ya que el código fue pensado para que el prop sea false o true, no para que vaya alternando; el resultado puede volverse caótico en tal situación

iTzGooDLife commented 2 months ago

Esto se dejó así ya que para hacer lo que tú dices se requeriría un prop extra

No, no lo requeriría, a continuación dejo un ejemplo de una pequeña implementación donde un botón varía el estado required que se usa en los TextArea:

image

image

iTzGooDLife commented 2 months ago

No recomiendo en absoluto hacer lo que mencionas ya que el código fue pensado para que el prop sea false o true, no para que vaya alternando; el resultado puede volverse caótico en tal situación

Por qué se volvería caótico? Fundamenta por favor. No veo ningún problema con que alterne con el método mencionado.

jllanosg commented 2 months ago

No recomiendo en absoluto hacer lo que mencionas ya que el código fue pensado para que el prop sea false o true, no para que vaya alternando; el resultado puede volverse caótico en tal situación

Por qué se volvería caótico? Fundamenta por favor. No veo ningún problema con que alterne con el método mencionado.

ya que el código fue pensado para que el prop sea false o true, no para que vaya alternando;

No digo que no sea posible, pero que puede volverse caótico ya que no lo hice pensando implementarlo de esa manera. Así que porfavor testear bien antes de utilizarlo así, para evitar futuros problemas

iTzGooDLife commented 2 months ago

No recomiendo en absoluto hacer lo que mencionas ya que el código fue pensado para que el prop sea false o true, no para que vaya alternando; el resultado puede volverse caótico en tal situación

En cuyo caso, se debería modificar, ya que afecta mucho a la UI, donde incluso nos dieron charlas de ello, que mencionaban justamente evitar esos colores por defecto si es posible.

Sealra commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

caverav commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

jllanosg commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

Por lo que he estado leyendo, lo mejor sería incluir el `en ellabel` (de forma manual) y al no rellenarlo e intentar enviar el formulario, activar el prop required para que genere la alerta visual de no haber completado el campo requerido.

Yo creo que la mejor opción es esta, ya que me parece un error cargar demasiado de lógica este tipo de componentes.

caverav commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

Por lo que he estado leyendo, lo mejor sería incluir el `en ellabel` (de forma manual) y al no rellenarlo e intentar enviar el formulario, activar el prop required para que genere la alerta visual de no haber completado el campo requerido.

* [Using an asterisk to mark required fields is an easy way to improve the usability of your forms. Only marking optional fields makes it difficult for people to fill out the form.](https://www.nngroup.com/articles/required-fields/)

Yo creo que la mejor opción es esta, ya que me parece un error cargar demasiado de lógica este tipo de componentes.

Creo que sí tiene sentido, ya que serviría para poner un asterisco rojo, y no hacerlo tan "hardcodeado" como poner el asterisco manualmente, además que, en serio cada vez que hay un campo requerido habrá que poner el prop y además el * manualmente? Ya está el prop, es muy sucio hacer eso, no veo razón por la cual hacer eso, si agregar en el componente el asterisco si hay un required es mucha lógica, créeme que agregar manualmente el asterisco en cada uno es más, y da pie a errores, como poner un campo requerido pero sin poner el asterisco.

Respecto al link, solo habla sobre por qué es bueno poner un asterisco, nada de agregarlo manualmente al usar un componente o nada parecido haciendo un símil.

jllanosg commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

Por lo que he estado leyendo, lo mejor sería incluir el `en ellabel` (de forma manual) y al no rellenarlo e intentar enviar el formulario, activar el prop required para que genere la alerta visual de no haber completado el campo requerido.

* [Using an asterisk to mark required fields is an easy way to improve the usability of your forms. Only marking optional fields makes it difficult for people to fill out the form.](https://www.nngroup.com/articles/required-fields/)

Yo creo que la mejor opción es esta, ya que me parece un error cargar demasiado de lógica este tipo de componentes.

Creo que sí tiene sentido, ya que serviría para poner un asterisco rojo, y no hacerlo tan "hardcodeado" como poner el asterisco manualmente, además que, en serio cada vez que hay un campo requerido habrá que poner el prop y además el * manualmente? Ya está el prop, es muy sucio hacer eso, no veo razón por la cual hacer eso, si agregar en el componente el asterisco si hay un required es mucha lógica, créeme que agregar manualmente el asterisco en cada uno es más, y da pie a errores, como poner un campo requerido pero sin poner el asterisco.

Respecto al link, solo habla sobre por qué es bueno poner un asterisco, nada de agregarlo manualmente al usar un componente o nada parecido haciendo un símil.

Entiendo, entonces quedaría agregar un prop (no se me ocurre un nombre) que agregue un * al label, ya que el prop required debe ser activado cuando no se rellene un campo requerido para dar esa advertencia visual.

caverav commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

Por lo que he estado leyendo, lo mejor sería incluir el `en ellabel` (de forma manual) y al no rellenarlo e intentar enviar el formulario, activar el prop required para que genere la alerta visual de no haber completado el campo requerido.

* [Using an asterisk to mark required fields is an easy way to improve the usability of your forms. Only marking optional fields makes it difficult for people to fill out the form.](https://www.nngroup.com/articles/required-fields/)

Yo creo que la mejor opción es esta, ya que me parece un error cargar demasiado de lógica este tipo de componentes.

Creo que sí tiene sentido, ya que serviría para poner un asterisco rojo, y no hacerlo tan "hardcodeado" como poner el asterisco manualmente, además que, en serio cada vez que hay un campo requerido habrá que poner el prop y además el * manualmente? Ya está el prop, es muy sucio hacer eso, no veo razón por la cual hacer eso, si agregar en el componente el asterisco si hay un required es mucha lógica, créeme que agregar manualmente el asterisco en cada uno es más, y da pie a errores, como poner un campo requerido pero sin poner el asterisco. Respecto al link, solo habla sobre por qué es bueno poner un asterisco, nada de agregarlo manualmente al usar un componente o nada parecido haciendo un símil.

Entiendo, entonces quedaría agregar un prop (no se me ocurre un nombre) que agregue un * al label, ya que el prop required debe ser activado cuando no se rellene un campo requerido para dar esa advertencia visual.

Acabo de revisar el componente, creo que está pésimo utilizar el prop required para que cuando sea true se active el ring y cuando sea false no, el required debería indicar si el campo es requerido o no, de hecho, viendo, no estás poniéndole el campo required al objeto html en sí (<input>), también es ideal para el *, y por último llamar al de la alerta algo como requiredAlert, incluso como una función tal vez

jllanosg commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

Por lo que he estado leyendo, lo mejor sería incluir el `en ellabel` (de forma manual) y al no rellenarlo e intentar enviar el formulario, activar el prop required para que genere la alerta visual de no haber completado el campo requerido.

* [Using an asterisk to mark required fields is an easy way to improve the usability of your forms. Only marking optional fields makes it difficult for people to fill out the form.](https://www.nngroup.com/articles/required-fields/)

Yo creo que la mejor opción es esta, ya que me parece un error cargar demasiado de lógica este tipo de componentes.

Creo que sí tiene sentido, ya que serviría para poner un asterisco rojo, y no hacerlo tan "hardcodeado" como poner el asterisco manualmente, además que, en serio cada vez que hay un campo requerido habrá que poner el prop y además el * manualmente? Ya está el prop, es muy sucio hacer eso, no veo razón por la cual hacer eso, si agregar en el componente el asterisco si hay un required es mucha lógica, créeme que agregar manualmente el asterisco en cada uno es más, y da pie a errores, como poner un campo requerido pero sin poner el asterisco. Respecto al link, solo habla sobre por qué es bueno poner un asterisco, nada de agregarlo manualmente al usar un componente o nada parecido haciendo un símil.

Entiendo, entonces quedaría agregar un prop (no se me ocurre un nombre) que agregue un * al label, ya que el prop required debe ser activado cuando no se rellene un campo requerido para dar esa advertencia visual.

Acabo de revisar el componente, creo que está pésimo utilizar el prop required para que cuando sea true se active el ring y cuando sea false no, el required debería indicar si el campo es requerido o no, de hecho, viendo, no estás poniéndole el campo required al objeto html en sí (<input>), también es ideal para el *, y por último llamar al de la alerta algo como requiredAlert, incluso como una función tal vez

Pero encuentras pésima la implementación o solo el nombre del prop? Porque en ese caso es cosa de cambiar a otro nombre y agregar también el prop required

caverav commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

Por lo que he estado leyendo, lo mejor sería incluir el `en ellabel` (de forma manual) y al no rellenarlo e intentar enviar el formulario, activar el prop required para que genere la alerta visual de no haber completado el campo requerido.

* [Using an asterisk to mark required fields is an easy way to improve the usability of your forms. Only marking optional fields makes it difficult for people to fill out the form.](https://www.nngroup.com/articles/required-fields/)

Yo creo que la mejor opción es esta, ya que me parece un error cargar demasiado de lógica este tipo de componentes.

Creo que sí tiene sentido, ya que serviría para poner un asterisco rojo, y no hacerlo tan "hardcodeado" como poner el asterisco manualmente, además que, en serio cada vez que hay un campo requerido habrá que poner el prop y además el * manualmente? Ya está el prop, es muy sucio hacer eso, no veo razón por la cual hacer eso, si agregar en el componente el asterisco si hay un required es mucha lógica, créeme que agregar manualmente el asterisco en cada uno es más, y da pie a errores, como poner un campo requerido pero sin poner el asterisco. Respecto al link, solo habla sobre por qué es bueno poner un asterisco, nada de agregarlo manualmente al usar un componente o nada parecido haciendo un símil.

Entiendo, entonces quedaría agregar un prop (no se me ocurre un nombre) que agregue un * al label, ya que el prop required debe ser activado cuando no se rellene un campo requerido para dar esa advertencia visual.

Acabo de revisar el componente, creo que está pésimo utilizar el prop required para que cuando sea true se active el ring y cuando sea false no, el required debería indicar si el campo es requerido o no, de hecho, viendo, no estás poniéndole el campo required al objeto html en sí (<input>), también es ideal para el *, y por último llamar al de la alerta algo como requiredAlert, incluso como una función tal vez

Pero encuentras pésima la implementación o solo el nombre del prop? Porque en ese caso es cosa de cambiar a otro nombre y agregar también el prop required

En parte ambas, por un lado la implementación, en que no se utiliza la propiedad required en el objeto html, y además que sería bueno utilizar otro nombre para el de la alerta

jllanosg commented 2 months ago

Creo que una posible opción amigable para el UX, sería mantener el ícono de requerido independiente del estado booleano del prop, y este prop únicamente maneje el borde rojo del componente. Así, en un inicio no poseería este borde rojo por defecto, sin embargo sería posible renderizar este borde rojo al catchear un error (o cualquier acción que lo requiera).

En ese caso, lo mejor diría que es mantener un caracter * indicando que es obligatorio, el ícono representa una alarma, que en caso de tener rellenado el campo, no debería "lanzarse", estoy de acuerdo en mantener algo representando que es obligatorio, pero no el ícono de exclamación

Por lo que he estado leyendo, lo mejor sería incluir el `en ellabel` (de forma manual) y al no rellenarlo e intentar enviar el formulario, activar el prop required para que genere la alerta visual de no haber completado el campo requerido.

* [Using an asterisk to mark required fields is an easy way to improve the usability of your forms. Only marking optional fields makes it difficult for people to fill out the form.](https://www.nngroup.com/articles/required-fields/)

Yo creo que la mejor opción es esta, ya que me parece un error cargar demasiado de lógica este tipo de componentes.

Creo que sí tiene sentido, ya que serviría para poner un asterisco rojo, y no hacerlo tan "hardcodeado" como poner el asterisco manualmente, además que, en serio cada vez que hay un campo requerido habrá que poner el prop y además el * manualmente? Ya está el prop, es muy sucio hacer eso, no veo razón por la cual hacer eso, si agregar en el componente el asterisco si hay un required es mucha lógica, créeme que agregar manualmente el asterisco en cada uno es más, y da pie a errores, como poner un campo requerido pero sin poner el asterisco. Respecto al link, solo habla sobre por qué es bueno poner un asterisco, nada de agregarlo manualmente al usar un componente o nada parecido haciendo un símil.

Entiendo, entonces quedaría agregar un prop (no se me ocurre un nombre) que agregue un * al label, ya que el prop required debe ser activado cuando no se rellene un campo requerido para dar esa advertencia visual.

Acabo de revisar el componente, creo que está pésimo utilizar el prop required para que cuando sea true se active el ring y cuando sea false no, el required debería indicar si el campo es requerido o no, de hecho, viendo, no estás poniéndole el campo required al objeto html en sí (<input>), también es ideal para el *, y por último llamar al de la alerta algo como requiredAlert, incluso como una función tal vez

Pero encuentras pésima la implementación o solo el nombre del prop? Porque en ese caso es cosa de cambiar a otro nombre y agregar también el prop required

En parte ambas, por un lado la implementación, en que no se utiliza la propiedad required en el objeto html, y además que sería bueno utilizar otro nombre para el de la alerta

OK

jllanosg commented 2 months ago

@caverav al final hice lo siguiente:

Esto solamente en SimpleInput, comentame que te parece y lo agrego al resto.