Skip to content

Comments

Develop#5

Open
xmaten wants to merge 19 commits intoD0man:developfrom
xmaten:develop
Open

Develop#5
xmaten wants to merge 19 commits intoD0man:developfrom
xmaten:develop

Conversation

@xmaten
Copy link

@xmaten xmaten commented Jan 19, 2019

No description provided.

main.js Outdated


const config = {
apiKey: 'AIzaSyAYWMXR8w8m16F7pVp_RZsnsBg1JkjYkVY',
Copy link

Choose a reason for hiding this comment

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

ojoj, nie podawaj tego ;)

użyj dotenv i plików .env

Copy link
Author

Choose a reason for hiding this comment

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

Tzn. wiem, że w przypadku pełnoprawnych aplikacji to zły pomysł, ale jeśli robię coś tak typowo testowo, to też powinienem to ukryć?

Copy link

Choose a reason for hiding this comment

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

w tej chwili mam Twój API Key, i każdy kto zobaczy to repo ma Twój API KEY ;P może Ci nabić dużo requestów, etc

Copy link

Choose a reason for hiding this comment

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

Nie podoba mi się za bardzo Twoje media queries, tzn. za dużo ich. Nie lepiej by było dać dla każdej opcji po jednym zamiast dla każdego tagu? Niezbyt to czytelnie moim zdaniem wygląda

Copy link
Author

Choose a reason for hiding this comment

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

Masz 100% racji. Wydaje mi się, że zrobiłem tak dlatego, że na codzień pracuje w stylusie i tam media mam zrobione za pomocą mixinów i są zagnieżdżane pod konkretnym elementem którego dotyczą, taka konwencja w firmie. W przypadku użycia ich w taki sposób IMO zwiększają czytelność, natomiast tu rzeczywiście może być gorzej.

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.

3 participants