okfn-brasil / vitimas-da-intolerancia

Conheça as vítimas de ódio político
https://vitimasdaintolerancia.org/
GNU Lesser General Public License v3.0
101 stars 10 forks source link

Mobile styles for #13 #15

Closed humrochagf closed 6 years ago

cuducos commented 6 years ago

Hi @humrochagf. I was just merging this branch now we have conflicts. Until 7f57363, commits are on master already. We have conflicts because we both edited the styles for the footer (I added 202b857 to master). Can you fix conflicts and let me know when you're done so I can properly code review and merge?

cuducos commented 6 years ago

BTW, my bad. I just re-read my previous message and it looks like more aggressive then I intended. I was just surprised that after pushing your comments plus 202b857 to master the PR hasn't shown itself as merged and thus I forgot the most important thing: a huge thank you for the collaboration, interest and stamina in working on this issue late night yesterday and early morning today ❤️

humrochagf commented 6 years ago

No problem, i will do the merge and fix the conflicts right now

humrochagf commented 6 years ago

@cuducos i fixed the conflicts and also changed the font approach to use a fluid font size math from https://css-tricks.com/snippets/css/fluid-typography/ so it will be better to read at any phone size

cuducos commented 6 years ago

Many thanks for advancing on this issue!

IMHO the current strategy over-complicated the CSS, @humrochagf. What about defining a style with this fluid-typography strategy once in the body { … } and then using rem to inherit this size in all other elements?

humrochagf commented 6 years ago

Yeah, i share this though. But i was thinking on do that on a second PR organizing it on maybe a sass structure to keep things on a kiss aproach

cuducos commented 6 years ago

We have a working mobile version online. I think there are two ways from now:

I see no urgency to merge something that will bring (more) technical debt in at this point.

humrochagf commented 6 years ago

Right, i will think i will go with the second first and then evolve to the first after

codecov[bot] commented 6 years ago

Codecov Report

Merging #15 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #15   +/-   ##
=======================================
  Coverage   92.21%   92.21%           
=======================================
  Files           4        4           
  Lines         167      167           
=======================================
  Hits          154      154           
  Misses         13       13

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7101e15...d796d78. Read the comment docs.

humrochagf commented 6 years ago

@cuducos done! Thank you with the advises :heart:

humrochagf commented 6 years ago

Sorry, now its done :smile: forgot to check big displays

cuducos commented 6 years ago

Awesome ✨