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

fix: Alignment Issue of Cards In Community Page #29644

Closed
wants to merge 1 commit into from
Closed

fix: Alignment Issue of Cards In Community Page #29644

wants to merge 1 commit into from

Conversation

CIPHERTron
Copy link
Member

@CIPHERTron CIPHERTron commented Sep 8, 2021

  • This PR fixes the alignment issues in kubernetes.io/community and introduces the following behavior:
k8s.mp4
  • Make each card of the same size

  • Minor refactor - Removed unnecessary CSS

  • However, there's still a lot to refactor in the community website in terms of CSS. For instance:

    • Having HTML & CSS separately i.e. avoiding the use of inline CSS
    • use of CSS Grids instead of 7-8 lines of CSS in the layout of items
    • remove repeated CSS

Closes #29155

@k8s-ci-robot
Copy link
Contributor

Welcome @CIPHERTron!

It looks like this is your first PR to kubernetes/website 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/website has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 8, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign celestehorgan after the PR has been reviewed.
You can assign the PR to them by writing /assign @celestehorgan in a comment when ready.

The full list of commands accepted by this bot can be found 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 language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Sep 8, 2021
@netlify
Copy link

netlify bot commented Sep 8, 2021

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: a04e248

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/61390874eb70a70008e3e0d7

😎 Browse the preview: https://deploy-preview-29644--kubernetes-io-main-staging.netlify.app

@CIPHERTron CIPHERTron changed the title fix: Misalignment Issue In Community Page fix: Alignment Issue of Cards In Community Page Sep 8, 2021
@sftim
Copy link
Contributor

sftim commented Sep 10, 2021

/area web-development

@k8s-ci-robot k8s-ci-robot added the area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes label Sep 10, 2021
@celanthe
Copy link
Contributor

@CIPHERTron I had a chance to review this, and it looks good to me!

Note for @sftim I'm in the process of trying to get REVIEWER status in #contribex-mentoring. Is there something I need to do officially to get this review to count towards my review count? The changes proposed in the PR seem solid to me, and pass checks. :) Is there anything else needed from me to help @CIPHERTron get this merged?

Thanks for any help you can provide!

@chrismetz09
Copy link
Contributor

@CIPHERTron, looks okay to me. Just to clarify, your remark, "refactor in the community website in terms of CSS" is an observation to address in a future PR?

@CIPHERTron
Copy link
Member Author

@CIPHERTron, looks okay to me. Just to clarify, your remark, "refactor in the community website in terms of CSS" is an observation to address in a future PR?

Yes @chrismetz09, I plan to address it in a future PR. I've opened an issue regarding the same here.

@CIPHERTron
Copy link
Member Author

CIPHERTron commented Sep 19, 2021

Heyy @chrismetz09 can we wrap up this PR? Thanks!

@kbhawkey
Copy link
Contributor

👀

@RinkiyaKeDad
Copy link
Member

@chrismetz09 can you please /lgtm this if the changes look okay to you 🙂

@sftim
Copy link
Contributor

sftim commented Oct 3, 2021

@CIPHERTron I found this change hard to review because there are lots of changes here.

It looks like you've combined a refactoring / tidying change with an actual fix? (I'm not sure).

If you can talk reviewers through what you've changed and why, that will help us get this reviewed and merged.

@kbhawkey
Copy link
Contributor

kbhawkey commented Oct 3, 2021

@kbhawkey
Copy link
Contributor

kbhawkey commented Oct 3, 2021

Hi @CIPHERTron . Thanks for contributing.
Yes, I would prefer a separation of layout changes from changes that tidy (remove extra lines or spaces, quotes) the source file.
Also, I am having trouble pinpointing the main change. What changes should I look for on the page?
When I resize my screen to tablet or phone size, some of the images go out of view and the text overflows in the top navigation. The "Community Values" section shows two images instead of three images?

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2021
@k8s-ci-robot
Copy link
Contributor

@CIPHERTron: PR needs rebase.

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.

@jimangel
Copy link
Member

jimangel commented Dec 6, 2021

@CIPHERTron thanks for the PR! I think it would be best to separate your changes into individual PRs (at the very least different commits) for ease of review. Given the age of the PR without action, I'm going to close, but feel free to reopen when ready to address feedback / conflicts. Thanks!

/close

@k8s-ci-robot
Copy link
Contributor

@jimangel: Closed this PR.

In response to this:

@CIPHERTron thanks for the PR! I think it would be best to separate your changes into individual PRs (at the very least different commits) for ease of review. Given the age of the PR without action, I'm going to close, but feel free to reopen when ready to address feedback / conflicts. Thanks!

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misalignment in /community, on mobile browsers.
8 participants