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

added Linkedln page link #5976

Merged
merged 5 commits into from
Jun 11, 2024

Conversation

shivamgupta2020
Copy link
Contributor

Fixes #5972

Proposed Changes

  • Added Linkedln page link in Follow us section.
    image

@knative-prow knative-prow bot requested a review from pmbanugo May 15, 2024 21:44
@knative-prow knative-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 15, 2024
@knative-prow knative-prow bot requested a review from skonto May 15, 2024 21:44
Copy link

netlify bot commented May 15, 2024

Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 7f2b53d
🔍 Latest deploy log https://app.netlify.com/sites/knative/deploys/6662321e38997c0007fb84d2
😎 Deploy Preview https://deploy-preview-5976--knative.netlify.app
📱 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.

@knative-prow knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 16, 2024
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.

Can you please revert the formatting changes that are not relevant to the task?

Also, can you add some spacing between X and LinkedIn logos?

@DhairyaMajmudar
Copy link

I think it will look much better if we split the logos section into 4 columns. One column for each logo.

Also while hovering it shows the logo into greenish gradient which can be made better if we change the color into the actual logo color (egs: black for Twitter logo) or we can also change the logo color into color matching Knative logos while hovering.

cc: @aliok @shivamgupta2020

@knative-prow knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 28, 2024
@shivamgupta2020
Copy link
Contributor Author

Can you please revert the formatting changes that are not relevant to the task?

Also, can you add some spacing between X and LinkedIn logos?

@aliok I have updated the linting of code and add some space between the logo.
Updated Look -
Screenshot from 2024-05-29 04-47-02

@shivamgupta2020
Copy link
Contributor Author

I think it will look much better if we split the logos section into 4 columns. One column for each logo.

Also while hovering it shows the logo into greenish gradient which can be made better if we change the color into the actual logo color (egs: black for Twitter logo) or we can also change the logo color into color matching Knative logos while hovering.

cc: @aliok @shivamgupta2020

Hi @DhairyaMajmudar ,
I think splitting the section into four parts isn't necessary since LinkedIn and Twitter perform the same task. Also, the color change on hover hasn't been decided by the UX team yet, so we should hold off on that for now. However, I like your suggestion to use the actual logo color on hover.

@aliok
Copy link
Member

aliok commented Jun 6, 2024

@shivamgupta2020

If you remove the irrelevant formatting changes, we can merge this PR. Thanks!

@shivamgupta2020
Copy link
Contributor Author

@shivamgupta2020

If you remove the irrelevant formatting changes, we can merge this PR. Thanks!

@aliok, I've reverted the formatting changes. Please review it on your machine.

@aliok
Copy link
Member

aliok commented Jun 6, 2024

@shivamgupta2020

When I check https://github.com/knative/docs/pull/5976/files I see they are still there.

@shivamgupta2020
Copy link
Contributor Author

@aliok, I've reverted the formatting changes. Please review it.

@nainaz
Copy link
Contributor

nainaz commented Jun 10, 2024

Looks good to me. I like the space between X and in.
Maybe it is just my eyes, in looks smaller like this :)
Thank you again @shivamgupta2020!

@aliok
Copy link
Member

aliok commented Jun 11, 2024

Thanks @shivamgupta2020

It looks great.

About the previous comments about colors, etc. Let's address them separately as we need to change those everywhere.

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.

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2024
Copy link

knative-prow bot commented Jun 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Jun 11, 2024
@knative-prow knative-prow bot merged commit 27e6e5f into knative:main Jun 11, 2024
19 checks passed
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We should add our LinkedIn page link to follow us section of our website
4 participants