Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Community page, as designed by @alexcontini #13046

Merged
merged 2 commits into from
Apr 26, 2019
Merged

New Community page, as designed by @alexcontini #13046

merged 2 commits into from
Apr 26, 2019

Conversation

cjyabraham
Copy link
Contributor

This new Community page was designed over the last few months by @alexcontini. I helped implement the Upcoming Events part of it.

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. language/en Issues or PRs related to English language size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 9, 2019
@netlify
Copy link

netlify bot commented Mar 9, 2019

Deploy preview for kubernetes-io-master-staging ready!

Built with commit bb1fa69

https://deploy-preview-13046--kubernetes-io-master-staging.netlify.com

@cjyabraham
Copy link
Contributor Author

CLA now signed.
/assign @jaredbhatti

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 9, 2019
@jaredbhatti
Copy link
Contributor

Thanks, I was pinged by acontini to hold off on approval until he pings me with a few updates.

.banner1 {
position:relative;
float:left;

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
width: 100%;

Maybe it will be good to add a width of 100% to the .banner1 as well. Otherwise the image can't be full width.

@rlenferink
Copy link
Member

What is the status on this? As I really like the newly designed community page :D

@alexcontini
Copy link
Contributor

@parispittman @castrojo this is the PR for the new community page (test page here - https://deploy-preview-13046--kubernetes-io-master-staging.netlify.com/community/)

We are still waiting on some things before merging right? just wanted to double check!

@mdlinville
Copy link
Contributor

/hold

(until the waiting-for-things are done)

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Mar 26, 2019
@alexcontini
Copy link
Contributor

@parispittman @castrojo here is the updated preview page with Paris' image and text updates - https://deploy-preview-13046--kubernetes-io-master-staging.netlify.com/community/

let me know if all looks good when you have a chance!

@parispittman
Copy link
Contributor

parispittman commented Apr 15, 2019

some folks are reporting that the image fencing with the descriptions of our comms channels are in and out of the box with resizing of web browser.

also - the code of conduct, videos, etc toolbar is not working for some folks

Screen Shot 2019-04-15 at 9 24 06 AM

thank you for your help with everything!!

@jonasrosland
Copy link
Contributor

Same comment as @parispittman above, the scaling is a little off in those boxes one larger screens.

For the events, can we organize the events so that it's spread out across multiple lines instead of being just one line?
Going from this:
image
to

Event
Date
Location

After clicking the links in the toolbar, the drop-down menu obscures the titles:

image

image

image

image

Copy link
Contributor

@jonasrosland jonasrosland left a comment

Choose a reason for hiding this comment

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

Found two small things that hopefully can be looked at.

<div class="intro">

<p>The Kubernetes community -- users, contributors, and the culture we've built together -- is one of the biggest reasons for the meteoric rise of this open source project. Our culture and values continue to grow and change as the project itself grows and changes. We all work together toward constant improvement of the project and the ways we work on it.
<br><br>We are the people who file issues and pull requests, attend SIG meetings, Kubernetes meetups, and KubeCon, advocate for it's adoption and innovation, run <code>kubectrl get pods</code>, and contribute in a thousand other vital ways. Read on to learn how you can get involved and become part of this amazing&nbsp;community.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be kubectl get pods, right?
And I think we can remove &nbsp; on the last line.


<h1 style="margin-top:0px">Videos</h1>

<div style="margin-bottom:4%;font-weight:300;text-align:center;padding-left:10%;padding-right:10%">We're on youtube, a lot. Subscribe for a wide range of&nbsp;topics.</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be YouTube, and I think we can remove the &nbsp; here too.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Apr 22, 2019
@alexcontini
Copy link
Contributor

@jonasrosland @parispittman @mrbobbytables

I have made the fixes, when you have a chance, will you please take a look at https://deploy-preview-13046--kubernetes-io-master-staging.netlify.com/community/ and let me know if all looks good now.

The only one I was unsure about it the anchor links, they were working for me in both browsers so it was hard to figure out the issue, i did change something though so let me know if not fixed now. I also moved up the anchors so they shouldn't cut off the titles anymore hopefully

@mrbobbytables
Copy link
Member

@alexcontini That fixed it for me :)
Thank you!

@jonasrosland
Copy link
Contributor

Hi @alexcontini!

Thank you for working on this, it's looking really good :)

The site still says kubectrl get pods, it should be kubectl get pods.

The squares at the bottom aren't uniform, but the text alignment looks much better :)

Large screen:
image

Smaller screen:
image

I see the events are now split up into

Event
Location - Time

It looks a little strange still:
image

Could it be set up on 3 lines without any -? Like this:

Event
Location
Time

The anchor links looks better but still seems to require a little trimming:
CoC:
image

Videos:
image

Discussions now have a big block above it compared to before:
image

Same with events:
image

This is in Safari on latest Mac OSX, and the same happens in Chrome.

@mrbobbytables
Copy link
Member

@jonasrosland The event data is sourced from an external calendar. It'd be fixing it there.
The anchor links look okay for me on chrome (Version 73.0.3683.103 (Official Build) (64-bit)) on OSX.

The boxes do shift size when resizing.

@mrbobbytables
Copy link
Member

Okay, was able to replicate the anchor thing, but it was only when I had the page sized a bit bigger than above where it flips to mobile menu form.

@alexcontini
Copy link
Contributor

@mrbobbytables @jonasrosland
I have made some fixes: https://deploy-preview-13046--kubernetes-io-master-staging.netlify.com/community/

  1. The resource box alignment - I added a min-height, but it's been tricky to make it match perfectly at every width because the text is such different lengths in each box (that is why I set the height before, but then it was getting cut off) - let me know if this looks any better to you!

  2. The anchor links - I added more spacing above the titles at the width where the menu becomes the mobile menu, so the issue before with the words getting cut off shouldn't happen anymore, let me know otherwise.

  3. The event section text - @mrbobbytables mentioned this above but it's all feeding through a google calendar so I can't edit the way the text looks. I will ask someone on the team if they can change that though

Let me know if this looks good to you when you have a chance, and thank you for all the feedback!

@jonasrosland
Copy link
Contributor

Looks really good @alexcontini ! Thanks for those fixes, seems like it works great now.
The calendar piece is more of a nice-to-have than anything else, so I think that could be worked on afterwards.

Looks like you might need to squash a few commits to get rid of the nags from the bot though :)

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2019
@zacharysarah
Copy link
Contributor

zacharysarah commented Apr 26, 2019

@cjyabraham @alexcontini This looks so good! ✨

Some feedback:

  1. There's a merge conflict to resolve.
  2. Please squash your commits by rebasing.
  3. This PR is old enough that the preview is still displaying 1.13 as the latest version. That's not a problem, since there's no merge conflict with the version selector. Rebasing should pull in the latest changes and update the preview.

@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. labels Apr 26, 2019
@cjyabraham
Copy link
Contributor Author

I have squashed and rebased the commits. The merge conflict brought in the "Talk to Us" section that you can see on the live site. It is now poorly formatted and also seems to have hidden our embedded twitter widget. See preview.

@alexcontini can you fix this please?

@alexcontini
Copy link
Contributor

@zacharysarah thank you! above issue is fixed (https://deploy-preview-13046--kubernetes-io-master-staging.netlify.com/community/) and all else should be ready now, are you able to merge when you have a chance?

@parispittman
Copy link
Contributor

Thanks @alexcontini and @cjyabraham for all of your hard work :)

@zacharysarah
Copy link
Contributor

/approve

@zacharysarah
Copy link
Contributor

/lgtm

@alexcontini /hold cancel when ready to merge.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zacharysarah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 26, 2019
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 26, 2019
@alexcontini
Copy link
Contributor

/hold cancel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants