BustByte / coronastatus

Anonymous crowd sourcing of COVID-19 symptoms all over the world (with public data sets)!
MIT License
174 stars 89 forks source link

Form design tweaks #428

Closed jinjie closed 4 years ago

jinjie commented 4 years ago

393

jinjie commented 4 years ago

Question: Why am I seeing so many commits in one PR? Those were already merged into master. Am I doing anything wrong?

fossecode commented 4 years ago

Question: Why am I seeing so many commits in one PR? Those were already merged into master. Am I doing anything wrong?

It is probably because the commit id's change when we squash and merge, so git does not understand that the commits are already on master. You could reset your fork after a PR has been merged, https://stackoverflow.com/questions/9646167/clean-up-a-fork-and-restart-it-from-the-upstream

jinjie commented 4 years ago

Thank you for the explanation.

Regards, Kong Jin Jie

On Sat, 28 Mar 2020, 5:44 PM Eirik Fosse, notifications@github.com wrote:

@fossecode approved this pull request.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BustByte/coronastatus/pull/428#pullrequestreview-383298860, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUYLKZWHV7V2SAKNDT2373RJXBI7ANCNFSM4LVNFIXA .

fossecode commented 4 years ago

@jinjie do you have time to complete this? 😄

jinjie commented 4 years ago

Report.ejs - line 211 / 408 - change py-2 to py-3

Line numbers are different, but I figured that it is for postal and temperature field.

Report.ejs - line 81, add class="appearance-none rounded focus:outline-none focus:shadow-outline"

Don't understand this instruction, not sure if the line numbers are different.

amritnagi commented 4 years ago

Hey, i meant just adding the classes to this element <select required name="age" id="Age">

so: <select class="appearance-none rounded focus:outline-none focus:shadow-outline" required name="age" id="Age">

amritnagi commented 4 years ago

@fossecode - do you know who did the custom checkboxes?
Could you ask them to look at adding focus styles on them? As you don't see which element is selected when tabbing through using the keyboard (and i couldn't get it to work)

jinjie commented 4 years ago

Just wondering will this work for the focus?

Screenshot 2020-03-30 at 3 11 48 PM
amritnagi commented 4 years ago

YES!! Can you get that to work on the checkboxes too?

jinjie commented 4 years ago

YES!! Can you get that to work on the checkboxes too?

Working on it... Checkbox seems a little tricky.

jinjie commented 4 years ago

Please check. 😄

fossecode commented 4 years ago

@fossecode - do you know who did the custom checkboxes? Could you ask them to look at adding focus styles on them? As you don't see which element is selected when tabbing through using the keyboard (and i couldn't get it to work)

Not really sure, we just used a template in the first version. Don't know if you guys changed them when implementing the new design?

fossecode commented 4 years ago

Should we merge this? 😄 @amritnagi

amritnagi commented 4 years ago

All good from me! :)

fossecode commented 4 years ago

Awesome, great work @jinjie