Hacktoberfest / hacktoberfest-2020

Hacktoberfest - App to manage the annual open-source challenge, used for the 2019 & 2020 seasons.
https://hacktoberfest.digitalocean.com
Other
496 stars 147 forks source link

Added CTA slim banner for updates page above navigation header #617

Closed pandafy closed 3 years ago

pandafy commented 3 years ago

Description

While browsing the website, I was not able to find how to get to the updates page. I remembered visiting the page from a post on Twitter. It does not feel right in my opinion to have a page as important as updates hidden from the users. I had to find the post on Twitter just to share this page with others, Let's save other users from such hassle.

Test process

I was not able to find tests for elements on static website. I would like to add them if you can point me where they are. Thanks!

Requirements to merge

pandafy commented 3 years ago

Thanks for you input Matt!

a slim CTA banner above the navigation

I would like to complete this PR but I don't have the exact idea from UI perspective, how it should look. It will be helpful it you can share any design document you folk have created for that. Otherwise, I would have to take a jab at it and create something which makes sense. Any references to implementation on other websites will also work,

MattIPv4 commented 3 years ago

I'm absolutely not a designer and I don't see anything in our Figma file at present, so I think you can just take a stab at it -- here's a quick idea I mocked:

image

(The Hacktoberfest is now officially opt-in only for projects and maintainers copy is what has been requested from our team -- the banner should show on all pages and link to the update post page)

Thanks again! :)

pandafy commented 3 years ago

Hnet-image (27)

How does it look @MattIPv4?

Also these lines does makes sense to me https://github.com/digitalocean/hacktoberfest/blob/deb1961a50db165a54342dad506d24caa264f3d3/app/assets/stylesheets/header.scss#L17-L20

I referenced MDN and it does not seems the right way to define values to padding-inline-start property. Can you help me with this, I think this line could be removed.

pandafy commented 3 years ago

Thanks for the detailed review @MattIPv4. Do you think we can keep "Updates" tab on navigation header? I know it seems redundant, but on updates page, user seems unaware of where there at from navigation header. We can either selectively show that Updates tab in header only on updates page. What do you think?

MattIPv4 commented 3 years ago

@pandafy sorry for the delayed response -- yes, welcome to also add a nav item, and yes, might as well remove that bad css in this PR!

pandafy commented 3 years ago

No hurries @MattIPv4 :wink: I have added nav item for Updates page and removed bad CSS in new commit. :smile:

pandafy commented 3 years ago

I don't know how that gap sneaked passed my self-review. I have fixed that. :sweat_smile:

I really like the idea of having text underlined on hover of link, it really fills up the lack of colour contrast between normal and hover colours. Did you aim for subtle colour shifts for hover event?

MattIPv4 commented 3 years ago

Yeah, those colors were supplied by our design team -- this all looks good to me, just waiting for internal approval before we merge :)