-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: implements navbar #16
Conversation
arufonsekun
commented
Feb 15, 2021
•
edited
Loading
edited
- closes Implementar Navbar #10
src/components/Navbar/index.jsx
Outdated
@@ -0,0 +1,36 @@ | |||
import React, {Component} from "react"; | |||
import "./navbar.scsss"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opa, hei de arrumar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A navbar ficou bem bonita, no entanto ocorreu uma sobreposição, talvez pela navbar estar transparente (não sei se foi intencional):
Além disso, adicionei alguns comentários/sugestões à respeito do layout de código. Levarei algumas discussões à issue #5.
src/components/Navbar/index.jsx
Outdated
|
||
return ( | ||
<nav className="navbar navbar-fixed"> | ||
<a href={home} className="nav-item"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acredito que deveríamos utilizar um componente Link
para isto, como indicado na documentação;
import "./navbar.scss"; | ||
|
||
class Navbar extends Component { | ||
render(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
De acordo com o style guide do airbnb, adicionar o seguinte espaçamento melhora a legibilidade:
render(){ | |
render () { |
src/pages/index.js
Outdated
@@ -10,6 +11,7 @@ export default class IndexPage extends Component { | |||
render() { | |||
return ( | |||
<div> | |||
<Navbar/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Também de acordo com o style guide do airbnb, seria "correto" utilizar o seguinte espaçamento:
<Navbar/> | |
<Navbar /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Apenas se formos seguir os padrões de código como na issue #5, há algumas inconsistências. Se as recomendações da issue estão incorretas, uma discussão deve ser levantada para encontrar padrões melhores e adotá-los pelo projeto todo.