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 pareja2 | Botones #3

Closed massi-ponce closed 3 months ago

massi-ponce commented 3 months ago

Agrega componentes genéricos de UI del tipo botones

IMPORTANTE: es necesario ejecutar npm install antes de correr la aplicación ya que para esta implementación se actualizaron los paquetes.

Ejemplos de uso componentes

DayPicker

Este componente funciona como un selector de fecha.

Cómo llamar al componente:

export const App = () => {
 ...
 const [day, setDay] = useState<Dayjs | null>(null);
 ...
 return (
 ...
  <div>
   <DayPicker
     label="Start Date"
     selectedDay={day}
     onChange={setDay}
   />
 </div>
 ...
 );
};

Se puede "setear" con una fecha inicial en particular o con null (para este ejemplo se utiliza null).

Vista en front-end:

imagen

Al clickear en el campo se puede introducir la fecha manualmente (siguiendo el formato dd/mm/yyyy). Al clickear el ícono del calendario se despliega un calendario para poder seleccionar un día en particular. Dentro de este calendario se puede cambiar el año (clickeando en él) y el mes (cambiando con las flechas que mueven de izquierda a derecha).

imagen

imagen

Radio y RadioGroup

Este componente funciona como un o muchos botones seleccionables del tipo radio genérico.

Cómo llamar al componente:

const RadioOptions = [
  { id: "1", label: "Opcion 1", value: "1" },
  { id: "2", label: "Opcion 2", value: "2" },
  { id: "3", label: "Opcion 3", value: "3", disabled: true },
];

export const App = () => {
 ...
 const [selectedValue, setSelectedValue] = useState("");
 ...
 return (
 ...
 <div>
  <RadioGroup
    name="example"
    options={RadioOptions}
    value={selectedValue}
    onChange={setSelectedValue}
  />
 </div>
 ...
 );
};

Como RadioGroup utiliza Radio, hay que pasarle la información de cada uno de los Radio que va a utilizar (por eso se crea una lista llamada RadioOptions).

Vista en front-end:

imagen

imagen

Como se definió en RadioOptions, se invocan 3 botones Radio. Cada uno tiene su label y el tercero está deshabilitado.

CheckboxButton

Este componente funciona como un botón seleccionable del tipo checkbox genérico.

Cómo llamar al componente:

export const App = () => {
 ...
 const [isChecked, setIsChecked] = useState(false);
 ...
 return (
 ...
  <div>
   <CheckboxButton
     text="Ejemplo Checkbox"
     checked={isChecked}
     onChange={setIsChecked}
   />
 </div>
 ...
 );
};

Vista en front-end:

imagen

imagen

jllanosg commented 3 months ago

Agregué lo siguiente a DayPicker:

interface DayPickerProps {
  ...
  onChange: React.Dispatch<React.SetStateAction<Dayjs>>;
}

...
const handleDayChange = (value: Dayjs | null) => {
    value && onChange(value);
  };

...
  return (
    <ThemeProvider theme={customTheme}>
      <LocalizationProvider dateAdapter={AdapterDayjs}>
        <Box sx={{ p: 2 }}>
          <DatePicker
           <---
            onChange={handleDayChange}
           --->
          />
        </Box>
      </LocalizationProvider>
    </ThemeProvider>
  );
};

Con este cambio, el estado del componente se puede controlar con un hook useState de la siguiente manera:

  const [day, setDay] = useState(dayjs().date(0));

  return (
    <>
      <DayPicker label="test" selectedDay={day} onChange={setDay} />
      {day.toString()}
    </>
  );

Por favor corregirme si malinterpreté algo.

caverav commented 3 months ago

No se documenta lo suficiente ni en el PR ni en el código para saber como utilizar el DayPicker, también falta mencionar que se debe hacer npm install.

No creo necesario mencionar que se debe hacer npm install, siempre que se quiera correr la aplicación se debería hacer si hay una actualización

caverav commented 3 months ago

Agregué lo siguiente a DayPicker:

interface DayPickerProps {
  ...
  onChange: React.Dispatch<React.SetStateAction<Dayjs>>;
}

...
const handleDayChange = (value: Dayjs | null) => {
    value && onChange(value);
  };

...
  return (
    <ThemeProvider theme={customTheme}>
      <LocalizationProvider dateAdapter={AdapterDayjs}>
        <Box sx={{ p: 2 }}>
          <DatePicker
           <---
            onChange={handleDayChange}
           --->
          />
        </Box>
      </LocalizationProvider>
    </ThemeProvider>
  );
};

Con este cambio, el estado del componente se puede controlar con un hook useState de la siguiente manera:

  const [day, setDay] = useState(dayjs().date(0));

  return (
    <>
      <DayPicker label="test" selectedDay={day} onChange={setDay} />
      {day.toString()}
    </>
  );

Por favor corregirme si malinterpreté algo.

No estoy entendiendo la necesidad del cambio, ya se puede controlar desde afuera tal como estaba según lo que probé, y con tus cambios no leo que cambie este comportamiento (no puedo probar ya que no estoy con el computador), si puedes explicar por qué crear una función handleDayChange aparte (consideramdo que no puede ser cambiado a null, ya que se debe seleccionar 1 sí o sí en los radio groups), y por qué este cambio cambiaría el estado de controlado o no, no logro entenderlo

caverav commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

jllanosg commented 3 months ago

No se documenta lo suficiente ni en el PR ni en el código para saber como utilizar el DayPicker, también falta mencionar que se debe hacer npm install.

No creo necesario mencionar que se debe hacer npm install, siempre que se quiera correr la aplicación se debería hacer si hay una actualización

Pero si se hace una actualización de los paquetes hay que mencionarlo

jllanosg commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

Más que nada los ejemplos son necesarios ya que no presentaron la entrega a tiempo (el miercoles)

jllanosg commented 3 months ago

Agregué lo siguiente a DayPicker:

interface DayPickerProps {
  ...
  onChange: React.Dispatch<React.SetStateAction<Dayjs>>;
}

...
const handleDayChange = (value: Dayjs | null) => {
    value && onChange(value);
  };

...
  return (
    <ThemeProvider theme={customTheme}>
      <LocalizationProvider dateAdapter={AdapterDayjs}>
        <Box sx={{ p: 2 }}>
          <DatePicker
           <---
            onChange={handleDayChange}
           --->
          />
        </Box>
      </LocalizationProvider>
    </ThemeProvider>
  );
};

Con este cambio, el estado del componente se puede controlar con un hook useState de la siguiente manera:

  const [day, setDay] = useState(dayjs().date(0));

  return (
    <>
      <DayPicker label="test" selectedDay={day} onChange={setDay} />
      {day.toString()}
    </>
  );

Por favor corregirme si malinterpreté algo.

No estoy entendiendo la necesidad del cambio, ya se puede controlar desde afuera tal como estaba según lo que probé, y con tus cambios no leo que cambie este comportamiento (no puedo probar ya que no estoy con el computador), si puedes explicar por qué crear una función handleDayChange aparte (consideramdo que no puede ser cambiado a null, ya que se debe seleccionar 1 sí o sí en los radio groups), y por qué este cambio cambiaría el estado de controlado o no, no logro entenderlo

Es más que nada para poder controlarlo usando solamente un hook useState, ya que como no agregaron ejemplo de uso está dificil saber.

Como se vé en el snippet que dejé, el componente DayPicker se invoca y se utiliza simplemente un useState para manejar su estado, no hay que crear funciones adicionales fuera de DayPicker.

Y por si acaso no dije que es o no controlado, simplemente pensé que quedaría mucho más simple su implementación si se utiliza de esa manera, ya que el componente que se está encapsulando dentro de DayPicker por lo que veo no acepta un setNombreState si no que una función como la de handleDayChange

caverav commented 3 months ago

No se documenta lo suficiente ni en el PR ni en el código para saber como utilizar el DayPicker, también falta mencionar que se debe hacer npm install.

No creo necesario mencionar que se debe hacer npm install, siempre que se quiera correr la aplicación se debería hacer si hay una actualización

Pero si se hace una actualización de los paquetes hay que mencionarlo

Por qué hay que mencionarlo? Es regla general hacer npm install antes de correr/buildear la app luego de una actualización, de todas maneras lo haremos, no cuesta mucho

jllanosg commented 3 months ago

No se documenta lo suficiente ni en el PR ni en el código para saber como utilizar el DayPicker, también falta mencionar que se debe hacer npm install.

No creo necesario mencionar que se debe hacer npm install, siempre que se quiera correr la aplicación se debería hacer si hay una actualización

Pero si se hace una actualización de los paquetes hay que mencionarlo

Por qué hay que mencionarlo? Es regla general hacer npm install antes de correr/buildear la app luego de una actualización, de todas maneras lo haremos, no cuesta mucho

Desconozco esa regla

caverav commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

Más que nada los ejemplos son necesarios ya que no presentaron la entrega a tiempo (el miercoles)

Qué tiene que ver el atraso? Además que quedamos en que se podría hacer una reunión si había dudas de los componentes, pero que se revisaría, diría que es bastante intuitivo usarlo, pero en caso de que sigas queriendo un ejemplo, quieres que pongamos un código de ejemplo? O una versión escrita en lenguaje natural de la explicación de los props?

caverav commented 3 months ago

No se documenta lo suficiente ni en el PR ni en el código para saber como utilizar el DayPicker, también falta mencionar que se debe hacer npm install.

No creo necesario mencionar que se debe hacer npm install, siempre que se quiera correr la aplicación se debería hacer si hay una actualización

Pero si se hace una actualización de los paquetes hay que mencionarlo

Por qué hay que mencionarlo? Es regla general hacer npm install antes de correr/buildear la app luego de una actualización, de todas maneras lo haremos, no cuesta mucho

Desconozco esa regla

Y dónde está la de mencionar en el PR que hay que correr npm install cada vez que cambian las dependencias? Nunca he visto eso en un proyecto OSS, creo que es más pertinente tener como regla que se corra npm install al probar cambios, si no hay actualizaciones es un proceso que demora milisegundos

jllanosg commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

Más que nada los ejemplos son necesarios ya que no presentaron la entrega a tiempo (el miercoles)

Qué tiene que ver el atraso? Además que quedamos en que se podría hacer una reunión si había dudas de los componentes, pero que se revisaría, diría que es bastante intuitivo usarlo, pero en caso de que sigas queriendo un ejemplo, quieres que pongamos un código de ejemplo? O una versión escrita en lenguaje natural de la explicación de los props?

El atraso tiene que ver, ya que habíamos planificado que el miércoles se entregaría una versión preliminar de esto para afinar detalles posteriormente, y que revisaríamos las entregas ese mismo día, cosa que no pasó porque lo entregaron parcialmente.

No podemos hacer una reunión cada vez que se atrasen con algo

Con un código de ejemplo basta

jllanosg commented 3 months ago

No se documenta lo suficiente ni en el PR ni en el código para saber como utilizar el DayPicker, también falta mencionar que se debe hacer npm install.

No creo necesario mencionar que se debe hacer npm install, siempre que se quiera correr la aplicación se debería hacer si hay una actualización

Pero si se hace una actualización de los paquetes hay que mencionarlo

Por qué hay que mencionarlo? Es regla general hacer npm install antes de correr/buildear la app luego de una actualización, de todas maneras lo haremos, no cuesta mucho

Desconozco esa regla

Y dónde está la de mencionar en el PR que hay que correr npm install cada vez que cambian las dependencias? Nunca he visto eso en un proyecto OSS, creo que es más pertinente tener como regla que se corra npm install al probar cambios, si no hay actualizaciones es un proceso que demora milisegundos

Es verdad que no existen reglas, pero no quita que el PR aporta poco y nada al entendimiento de los cambios, así que queda pendiente establecer normas para esto

caverav commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

Más que nada los ejemplos son necesarios ya que no presentaron la entrega a tiempo (el miercoles)

Qué tiene que ver el atraso? Además que quedamos en que se podría hacer una reunión si había dudas de los componentes, pero que se revisaría, diría que es bastante intuitivo usarlo, pero en caso de que sigas queriendo un ejemplo, quieres que pongamos un código de ejemplo? O una versión escrita en lenguaje natural de la explicación de los props?

El atraso tiene que ver, ya que habíamos planificado que el miércoles se entregaría una versión preliminar de esto para afinar detalles posteriormente, y que revisaríamos las entregas ese mismo día, cosa que no pasó porque lo entregaron parcialmente.

No podemos hacer una reunión cada vez que se atrasen con algo

Con un código de ejemplo basta

Eso no tiene que ver, porque como digo, es algo que ya se conversó, quedamos en que no habría reunión obligatoria, revisarían si entienden cómo usarlo y luego si hay dudas preguntan y tal vez hacemos una reu, y nadie mencionó los ejemplos, pero eso haremos ahora que lo dices

jllanosg commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

Más que nada los ejemplos son necesarios ya que no presentaron la entrega a tiempo (el miercoles)

Qué tiene que ver el atraso? Además que quedamos en que se podría hacer una reunión si había dudas de los componentes, pero que se revisaría, diría que es bastante intuitivo usarlo, pero en caso de que sigas queriendo un ejemplo, quieres que pongamos un código de ejemplo? O una versión escrita en lenguaje natural de la explicación de los props?

El atraso tiene que ver, ya que habíamos planificado que el miércoles se entregaría una versión preliminar de esto para afinar detalles posteriormente, y que revisaríamos las entregas ese mismo día, cosa que no pasó porque lo entregaron parcialmente. No podemos hacer una reunión cada vez que se atrasen con algo Con un código de ejemplo basta

Eso no tiene que ver, porque como digo, es algo que ya se conversó, quedamos en que no habría reunión obligatoria, revisarían si entienden cómo usarlo y luego si hay dudas preguntan y tal vez hacemos una reu, y nadie mencionó los ejemplos, pero eso haremos ahora que lo dices

Si, lo que pasa es que me costó caleta hacer funcionar el day picker así que tan intuitivo no fué

caverav commented 3 months ago

No se documenta lo suficiente ni en el PR ni en el código para saber como utilizar el DayPicker, también falta mencionar que se debe hacer npm install.

No creo necesario mencionar que se debe hacer npm install, siempre que se quiera correr la aplicación se debería hacer si hay una actualización

Pero si se hace una actualización de los paquetes hay que mencionarlo

Por qué hay que mencionarlo? Es regla general hacer npm install antes de correr/buildear la app luego de una actualización, de todas maneras lo haremos, no cuesta mucho

Desconozco esa regla

Y dónde está la de mencionar en el PR que hay que correr npm install cada vez que cambian las dependencias? Nunca he visto eso en un proyecto OSS, creo que es más pertinente tener como regla que se corra npm install al probar cambios, si no hay actualizaciones es un proceso que demora milisegundos

Es verdad que no existen reglas, pero no quita que el PR aporta poco y nada al entendimiento de los cambios, así que queda pendiente establecer normas para esto

Creo que este PR es el que más explicita los cambios hechos en comparación a los otros, pero concuerdo en que debemos establecer reglas para los PRs, y así estar en la misma página

caverav commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

Más que nada los ejemplos son necesarios ya que no presentaron la entrega a tiempo (el miercoles)

Qué tiene que ver el atraso? Además que quedamos en que se podría hacer una reunión si había dudas de los componentes, pero que se revisaría, diría que es bastante intuitivo usarlo, pero en caso de que sigas queriendo un ejemplo, quieres que pongamos un código de ejemplo? O una versión escrita en lenguaje natural de la explicación de los props?

El atraso tiene que ver, ya que habíamos planificado que el miércoles se entregaría una versión preliminar de esto para afinar detalles posteriormente, y que revisaríamos las entregas ese mismo día, cosa que no pasó porque lo entregaron parcialmente. No podemos hacer una reunión cada vez que se atrasen con algo Con un código de ejemplo basta

Eso no tiene que ver, porque como digo, es algo que ya se conversó, quedamos en que no habría reunión obligatoria, revisarían si entienden cómo usarlo y luego si hay dudas preguntan y tal vez hacemos una reu, y nadie mencionó los ejemplos, pero eso haremos ahora que lo dices

Si, lo que pasa es que me costó caleta hacer funcionar el day picker así que tan intuitivo no fué

Oka, si puedes explicar los inconvenientes que tuviste se agradece, para saber si realmente es un problema de ser intuitivo o no, no consideres lo de si es componente controlado o no, ya que eso se mencionó y es un error más que un indicador de lo intuitivo que puede ser, y es algo que ya no estará, esto para incluir esas instrucciones en el PR comentando el ejemplo

jllanosg commented 3 months ago

Faltan ejemplos de uso para el componente RadioGroup; aparentemente no es controlado por lo que debe ser modificado para conocer el valor seleccionado en un componente padre.

Efectivamente este componente no es controlado, es algo en lo que no me fijé al revisar el commit del Massi, simplemente probé que funcionara seleccionar solo 1 entre todos, es algo que hay que cambiar, respecto a ejemplos de uso, entendí que no se iban a utilizar ejemplos, es más, en los otros PRs, tanto el de ustedes como de la pareja 1 no se incluyen ejemplos de uso, solo se nombraron los componentes nuevos

Más que nada los ejemplos son necesarios ya que no presentaron la entrega a tiempo (el miercoles)

Qué tiene que ver el atraso? Además que quedamos en que se podría hacer una reunión si había dudas de los componentes, pero que se revisaría, diría que es bastante intuitivo usarlo, pero en caso de que sigas queriendo un ejemplo, quieres que pongamos un código de ejemplo? O una versión escrita en lenguaje natural de la explicación de los props?

El atraso tiene que ver, ya que habíamos planificado que el miércoles se entregaría una versión preliminar de esto para afinar detalles posteriormente, y que revisaríamos las entregas ese mismo día, cosa que no pasó porque lo entregaron parcialmente. No podemos hacer una reunión cada vez que se atrasen con algo Con un código de ejemplo basta

Eso no tiene que ver, porque como digo, es algo que ya se conversó, quedamos en que no habría reunión obligatoria, revisarían si entienden cómo usarlo y luego si hay dudas preguntan y tal vez hacemos una reu, y nadie mencionó los ejemplos, pero eso haremos ahora que lo dices

Si, lo que pasa es que me costó caleta hacer funcionar el day picker así que tan intuitivo no fué

Oka, si puedes explicar los inconvenientes que tuviste se agradece, para saber si realmente es un problema de ser intuitivo o no, no consideres lo de si es componente controlado o no, ya que eso se mencionó y es un error más que un indicador de lo intuitivo que puede ser, y es algo que ya no estará, esto para incluir esas instrucciones en el PR comentando el ejemplo

Como mencioné anteriormente tuve que agregar código extra aparte del useState al componente donde estaba usando el DayPicker. Por eso creé una función dentro de DayPicker, para adaptar el setState y simplificar el uso de DayPicker

jllanosg commented 3 months ago
const handleRadioGroupChange = (value: string) => {
  setSelectedValue(value);
 };
 const handleCheckboxChange = (value: boolean) => {
  setIsChecked(value);
 };

Es necesario hacer esta función cada vez que se quiere utilizar alguno de los componentes?

No es lo mismo que hice anteriormente?