Develop landing page landing page header #121

[KyleHefner]

Develop landing page page header.

[CSchmitz81]

For #121

[CSchmitz81]

This mostly looks good in terms of code, other than one suspicious thing. I still need to test it to verify the functionality.

[CSchmitz81]

Hmm, I'll need to check this out in practice, but it looks strange to essentially be 200% of the view height. Maybe I'm wrong, but I'll check this out when testing.

[CSchmitz81]

This is a good start, but there are still plenty of differences between the comps and the way this looks. Here are some of the bigger things I saw:

• The text size and margin sizes appear to be too small
• The menu button shows up for the desktop layout (it should only show up for the mobile layout)
• The selected text is bold black instead of green like the comp shows
• The layout doesn't appear to be themed for mobile at all (there are other things on the landing page which have a different mobile layout via CSS based on the screen width)
• There appears to be a regression where there are now 2 vertical scroll bars, probably because of that 200vh

[KyleHefner]

ok I will work on it again, I spent two hours trying to figure out how to change the selected text color to black to no avail, but I will try again.

…

On Wed, Sep 18, 2019 at 11:06 PM Chris Schmitz ***@***.***> wrote: ***@***.**** requested changes on this pull request. This is a good start, but there are still plenty of differences between the comps and the way this looks. Here are some of the bigger things I saw: - The text size and margin sizes appear to be too small - The menu button shows up for the desktop layout (it should only show up for the mobile layout) - The selected text is bold black instead of green like the comp shows - The layout doesn't appear to be themed for mobile at all (there are other things on the landing page which have a different mobile layout via CSS based on the screen width) - There appears to be a regression where there are now 2 vertical scroll bars, probably because of that 200vh — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#126?email_source=notifications&email_token=AECPVSPP2THWEQAKXT6YRJ3QKMJCJA5CNFSM4IWYUO22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFHDM3I#pullrequestreview-290338413>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AECPVSKGVRNMCEQFRY2EUTLQKMJCJANCNFSM4IWYUO2Q> .

[KyleHefner]

I mean green

…

On Thu, Sep 19, 2019 at 3:23 PM Kyle H ***@***.***> wrote: ok I will work on it again, I spent two hours trying to figure out how to change the selected text color to black to no avail, but I will try again. On Wed, Sep 18, 2019 at 11:06 PM Chris Schmitz ***@***.***> wrote: > ***@***.**** requested changes on this pull request. > > This is a good start, but there are still plenty of differences between > the comps and the way this looks. Here are some of the bigger things I saw: > > - The text size and margin sizes appear to be too small > - The menu button shows up for the desktop layout (it should only > show up for the mobile layout) > - The selected text is bold black instead of green like the comp shows > - The layout doesn't appear to be themed for mobile at all (there are > other things on the landing page which have a different mobile layout via > CSS based on the screen width) > - There appears to be a regression where there are now 2 vertical > scroll bars, probably because of that 200vh > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#126?email_source=notifications&email_token=AECPVSPP2THWEQAKXT6YRJ3QKMJCJA5CNFSM4IWYUO22YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFHDM3I#pullrequestreview-290338413>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AECPVSKGVRNMCEQFRY2EUTLQKMJCJANCNFSM4IWYUO2Q> > . >

[CSchmitz81]

If you can't figure something out you can just note that the implementation isn't complete and we can note it in the ticket and merge what we have so far. At least it will be closer to correct and we can continue working to try and resolve any lingering differences.

As far as the specific issue, I would first check if I could update the style by inspecting the CSS in the browser and changing it there (sometimes I need to mark a style as !important). Once I got that, I'd see what style changes it to bold and try to put the color change in the same place. If you still can't figure it out quickly feel free to leave it and make a note that you need help. It might also be a good idea to post somewhere online, especially our Slack group, to see if anyone can help. I'm always asking coworkers what they think about how I'm planning to implement things or suggestions for solving issues, so we should treat the Slack group the same way.

[CSchmitz81]

Is this something you're planning to work on during the hackathon this Saturday?

[CSchmitz81]

Merging, since progress has stalled, so we have the changes so far. Work will continue on #121.

[CSchmitz81]

This caused regressions, so reversing this merge.