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

Implemented sliding animation for the adopter logos #5878

Merged
merged 4 commits into from
Mar 5, 2024

Conversation

prajjwalyd
Copy link
Contributor

Addresses - knative/community#1416

Proposed Changes

I've been working on implementing the sliding animation for the adopters' logos.

scrnli_2_19_2024_1-16-44.AM.webm

I haven't used any external libraries for this...
Also, the logos along with their links are dynamically rendered as per the instructions.
Public preview link: https://prajjwalyd.github.io/docs/

added sliding animation to adopter logos

some cleanup
Copy link

linux-foundation-easycla bot commented Feb 23, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

knative-prow bot commented Feb 23, 2024

Welcome @prajjwalyd! It looks like this is your first PR to knative/docs 🎉

@knative-prow knative-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 23, 2024
@knative-prow knative-prow bot requested review from csantanapr and snneji February 23, 2024 11:59
Copy link

netlify bot commented Feb 23, 2024

Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit e93ab91
🔍 Latest deploy log https://app.netlify.com/sites/knative/deploys/65e754488fecd20008ab1541
😎 Deploy Preview https://deploy-preview-5878--knative.netlify.app/docs
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@aliok aliok left a comment

Choose a reason for hiding this comment

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

Thanks @prajjwalyd this looks great already!

I added some comments inline.

I just realized something though. If we add the adopters too (on top of current list of vendors), we might need a 2nd row. Ideally scrolling in the opposite direction, like CNCF does.

What do you think?

Comment on lines 323 to 330
const adopters = [
{ logoPath: "images/corporate-logos/Google.svg", url: "https://www.google.com" },
{ logoPath: "images/corporate-logos/vmware.svg", url: "https://www.vmware.com" },
{ logoPath: "images/corporate-logos/IBM.svg", url: "https://www.ibm.com" },
{ logoPath: "images/corporate-logos/Redhat.svg", url: "https://www.redhat.com" },
{ logoPath: "images/corporate-logos/triggermesh_logo.svg", url: "https://www.triggermesh.com" }

];
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add the adopters here?

https://github.com/knative/community/blob/main/ADOPTERS.MD

Unfortunately, we don't have their logos handy :(

Copy link
Member

Choose a reason for hiding this comment

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

And, can you make the whole list randomized? Not statically randomized, but randomized in the runtime?

I mean, it would be nice if we see the adopters+vendors in random order every time we refresh this page.

Copy link
Member

Choose a reason for hiding this comment

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

We should drop trigger mesh - they're no longer in business

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latest changes are as follows:

  1. Added more corporate logos by following - https://github.com/knative/community/blob/main/ADOPTERS.MD
  2. Mixed the adopters and vendors under the 'Trusted by' heading.
  3. Added a second row that scrolls to the right.
  4. Randomized the logo display order on each refresh.
  5. Dropped Trigger mesh.

Copy link
Contributor Author

@prajjwalyd prajjwalyd Mar 2, 2024

Choose a reason for hiding this comment

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

The current list is not yet fully completed, and there are some aspect ratio and styling fixes needed for certain images...
however, the logic has been implemented, and we simply need to continue adding the remaining logos and links to the list.
Let me know if this approach seems acceptable, and then I can proceed with the final styling adjustments!

Copy link
Member

Choose a reason for hiding this comment

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

Looks great!

As you said, some images need some work (aspect ratio, whitespace padding), but the overall logic looks good to me.

Special thanks for keeping the commit history clean!

I just noticed that padding is a little bit too much on mobile screens, but that's another story. Our website is not properly responsive.

@knative-prow knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 2, 2024
@prajjwalyd prajjwalyd requested review from aliok and dprotaso March 2, 2024 20:15
@Cali0707 Cali0707 self-requested a review March 5, 2024 17:29
Copy link
Member

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

/lgtm
/cc @aliok

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2024
@aliok
Copy link
Member

aliok commented Mar 5, 2024

/lgtm
/approve

Thanks a lot @prajjwalyd

As we talked in Slack, can you create a ticket for the missing logos?
And, also, for any other improvement that you've planned.

Copy link

knative-prow bot commented Mar 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok, prajjwalyd

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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 5, 2024
@knative-prow knative-prow bot merged commit 29695ed into knative:main Mar 5, 2024
19 checks passed
prushh pushed a commit to prushh/docs that referenced this pull request Apr 30, 2024
* added sliding animation to adopter logos

added sliding animation to adopter logos

some cleanup

* added some more corporate logos

* added a second row of adopters

* added/updated corporate logos and adjusted their styling
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. lgtm Indicates that a PR is ready to be merged. 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.

4 participants