-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add globals and resets #5
Conversation
✅ Deploy Preview for conejos-voladores-202309-bcn ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
package.json
Outdated
@@ -15,6 +15,7 @@ | |||
"dependencies": { | |||
"react": "^18.2.0", | |||
"react-dom": "^18.2.0", | |||
"styled-components": "^6.1.0" |
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.
cambiar a dependencia de desarrollo
src/styles/GlobalStyles.ts
Outdated
box-sizing: border-box; | ||
} | ||
|
||
h1,h2,body{ |
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.
falta un espacio entre los elmentos, así prittier te los pondrá en vertical
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.
también espacio antes de la llave
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.
debes intalar styled component para que estas cosas no ocurran o te avise de que no esta bien, hazlo y revisa los archivos una vez instalado
src/styles/GlobalStyles.ts
Outdated
const GlobalStyle = createGlobalStyle` | ||
*, | ||
::after, | ||
::before{ |
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.
espacio antes de la llave
src/styles/GlobalStyles.ts
Outdated
padding:0; | ||
} | ||
|
||
button{ |
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.
espacio antes de la llave
src/styles/GlobalStyles.ts
Outdated
padding: 0; | ||
} | ||
|
||
ul{ |
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.
espacio antes de la llave
src/styles/mainTheme.ts
Outdated
const mainTheme: DefaultTheme = { | ||
color: { | ||
main: "#fbd000", | ||
secondary: "#049cd8", |
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.
main-background
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.
falta un background #fff, el de los inputs del formulario. Aunque no sé si es yagni
input-background
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.
👍 🍸 💯 ;cool :okelevoyaponerinput
src/styles/mainTheme.ts
Outdated
main: "#fbd000", | ||
secondary: "#049cd8", | ||
"error-background": "#e52521", | ||
"check-background": "#43b047", |
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.
successfull-background, para que concorde con el figma
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.
falta arreglar éste
src/styles/mainTheme.ts
Outdated
secondary: "#049cd8", | ||
"error-background": "#e52521", | ||
"check-background": "#43b047", | ||
font: "#fff", |
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.
la blanca es la secondary-font-color, realmente se usa en los mensajes de server down de background rojo y en algun icon.
src/styles/mainTheme.ts
Outdated
"error-background": "#e52521", | ||
"check-background": "#43b047", | ||
font: "#fff", | ||
"secondary-font": "#000", |
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.
main-font-color, realmente se usa en toda la letra del documento
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.
Arreglar con la revisión de naming del mainTheme
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.
good now
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.
revisaremos la linea que sobra en la 37, pero es lo único que falta
index.html
Outdated
@@ -6,6 +6,7 @@ | |||
<link rel="shortcut icon" href="/favicon.webp" type="image/x-icon"> | |||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | |||
<meta name="Mario Smashers" content="Web about Mario Smashers characters"> | |||
<link href="https://fonts.cdnfonts.com/css/new-super-mario-font-u" rel="stylesheet"> |
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.
tienes que instalarte la dependencia de la fuente, no el link
} | ||
|
||
|
||
`; |
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.
sobran las líneas 35 y 36
const mainTheme: DefaultTheme = { | ||
color: { | ||
"container-background": "#fbd000", | ||
"main-background": "#049cd8", |
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.
ponlos por orden jerárquico: primero el main, después el container please, para que se entienda rápido
src/styles/mainTheme.ts
Outdated
main: "#fbd000", | ||
secondary: "#049cd8", | ||
"error-background": "#e52521", | ||
"check-background": "#43b047", |
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.
falta arreglar éste
src/styles/mainTheme.ts
Outdated
"secondary-font-family": "'New Super Mario Font U', sans-serif", | ||
size: "1,1875rem", |
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.
cambiar "size" a main-font-size
Añadir:
secondary-font-size: 2.875rem
form-title-font-size
form-input-font-size
pop-up: font-size
main: "#fbd000", | ||
secondary: "#049cd8", | ||
"container-background": "#fbd000", | ||
"main-background": "#049cd8", | ||
"error-background": "#e52521", | ||
"check-background": "#43b047", | ||
font: "#fff", | ||
"secondary-font": "#000", | ||
}, | ||
typography: { |
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.
falta un enter en la 12
"error-background": "#e52521", | ||
"check-background": "#43b047", | ||
font: "#fff", | ||
"secondary-font": "#000", | ||
}, | ||
typography: { | ||
"main-font-fammily": "Verdana, Geneva, Tahoma, sans-serif", | ||
"main-font-family": "Verdana, Geneva, Tahoma, sans-serif", | ||
"secondary-font-family": "'New Super Mario Font U', sans-serif", |
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.
instalar la fuente, no ponerlo en el html
src/styles/styled.d.ts
Outdated
main: string; | ||
secondary: string; | ||
"container-background": "#fbd000"; | ||
"main-background": "#049cd8"; |
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.
poner el main encima de container para respetar la jerarquía
src/styles/styled.d.ts
Outdated
main: string; | ||
secondary: string; | ||
"container-background": "#fbd000"; | ||
"main-background": "#049cd8"; | ||
"error-background": string; | ||
"check-background": string; |
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.
esto no es check, es successfull para que concorde con el figma
src/styles/styled.d.ts
Outdated
"secondary-font-family": string; | ||
size: string; |
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.
review las font-size
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
No description provided.