Skip to content

Atualização do modulo laboratório e suas respectivas telas#19

Open
02David03 wants to merge 12 commits into
developfrom
98
Open

Atualização do modulo laboratório e suas respectivas telas#19
02David03 wants to merge 12 commits into
developfrom
98

Conversation

@02David03
Copy link
Copy Markdown

-Atualização no css antigo do lab
-Criação da tela de listagem de laboratórios
-Funcionamento da edição, criação e exclusão do laboratório
-Criação da tela de visualização dos equipamentos pertencentes a um laboratório em especifico
-Equipamentos com exclusão funcionando

Copy link
Copy Markdown

@jonasbantunes jonasbantunes left a comment

Choose a reason for hiding this comment

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

Segue uma lista com as minhas sugestões de mudança, além das que fiz diretamente no GitHub. Elas aparecem mais de uma vez ao longo do código, então preferi listar aqui ao invés de comentar em cada trecho de código:

  • Remover console.log();
  • Passar o linter no projeto (prettier e eslint);
  • Atualizar a sintaxe de exportação para o padrão atual do JavaScript, usando export default ao invés de module.exports;
  • Mover as declarações de validationSchema para fora do corpo do componente.

Comment thread src/modules/Laboratorio/Editar/index.js Outdated
Comment thread src/modules/Laboratorio/Editar/index.js Outdated
Comment thread src/modules/Laboratorio/Editar/components/Formulario.js Outdated
Comment thread src/modules/Laboratorio/Listar/actions/selectionStatus.js Outdated
Comment thread src/modules/Laboratorio/Listar/actions/selectionStatus.js Outdated
Comment thread src/modules/Laboratorio/Visualizar/components/editar.js Outdated
Comment thread src/modules/Laboratorio/Visualizar/index.js Outdated
Comment thread src/modules/Laboratorio/Visualizar/index.js Outdated
Comment thread src/utils/components/ConfirmDelete.js Outdated
Comment thread src/utils/components/ModalDelete.js Outdated
Copy link
Copy Markdown

@jonasbantunes jonasbantunes left a comment

Choose a reason for hiding this comment

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

Sugestões de mudança:

  • Remover o package-lock.json e .yarn/releases/yarn-berry.cjs;
  • Remover os console.log() dos arquivos modificados no pull request;

OBS.: O yarn pode ser instalado através do npm, utilizando o comando npm install -g yarn.

};
const quantidade = `${Object.keys(labs).length} itens`;
const Listar = () => {
const [state, setState] = useState([]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Esse nome de variável não deixa claro do que se trata. Recomendo trocar por algo que indique se trata de uma lista de laboratórios, como labs ou laboratories, por exemplo.

Suggested change
const [state, setState] = useState([]);
const [labs, setLabs] = useState([]);

useEffect(() => {
async function fetchData() {
const res = await api.get('/laboratory');
setState(await res.data);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Esse await é redundante. A promessa (promise) do axios já foi finalizada com àwait e o res.data é um objeto, não uma promessa.

Suggested change
setState(await res.data);
setState(res.data);

name: values.name,
capacity: values.capacity,
is_in_use: false,
occupied_at: '2020-02-10T23:02:10.000Z',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Essa data de ocupação não deveria ser dinâmica?


import './styles/index.css';
const Listar = (props) => {
const [state, setState] = useState([]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Esse nome de variável não deixa claro do que se trata. Recomendo trocar por algo que indique se trata de um laboratório, como lab ou laboratory, por exemplo.

Suggested change
const [state, setState] = useState([]);
const [lab, setLab] = useState([]);


const ConfirmDelete = (props) => {

const [value,setValue] = useState();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Esse nome de variável não deixa claro do que se trata. Recomendo trocar por algo que indique se trata de do nome de um laboratório, como labName, laboratoryName ou mesmo name, por exemplo.

Suggested change
const [value,setValue] = useState();
const [labName, setLabName] = useState();

if (value === props.title){
api.delete(`/laboratory/${props.id}`);
alert('O laboratório ' + value + ' foi deletado!');
window.location.reload();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Comandos de roteamento nativo do navegador devem ser evitados, já que eles entram em conflito com o React Router e recarregam a página totalmente.

Suggested change
window.location.reload();

<div className="modalDelete">
<div className="modalDeleteBox" id="modal">
<div className="modalDeleteTitle">
<h2> {this.props.title} </h2>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ao alterar os props existentes deste componente, quem utiliza certamente terão as suas funcionalidades quebradas. Inclusive, a parte de Lista de Imagens de SO quebra com essa mudança.

Comment on lines +42 to +48
<ConfirmDelete
title={props.name}
id = {props.id}
onClose={showModal}
show={show}
>
</ConfirmDelete>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Os componentes ConfirmDelete e ModalDelete estão fortemente acoplados sem necessidade. Certas telas, como de Imagem de SO, apenas precisam do ModalDelete e não do ConfirmDelte. Idealmente, a exibição desses componentes deve ser orquestrado por quem quer utilizá-los, ao invés dos dois componentes se orquestrarem.

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