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

Changed the Twitter logo #9854

Closed
wants to merge 5 commits into from

Conversation

MayaSatishRao
Copy link
Contributor

@MayaSatishRao MayaSatishRao commented Jan 8, 2024

Resolve opencollective/opencollective#7195

Description

New Twitter (X) logo is added in the footer and header of open collective page

Screenshots

Screenshot 2024-01-08 200030
Screenshot 2024-01-08 200109

Copy link

vercel bot commented Jan 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
opencollective-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 1:18pm
opencollective-styleguide ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 30, 2024 1:18pm

@@ -138,7 +138,6 @@ const ContributorsGrid = ({
}) => {
const maxNbRows = maxNbRowsForViewports[viewport];
const [nbCols, nbRows] = getItemsRepartition(contributors.length, width, maxNbRows);

Copy link
Member

Choose a reason for hiding this comment

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

This change shouldn't be there either. I strongly recommend looking into https://jwiegley.github.io/git-from-the-bottom-up/1-Repository/8-interactive-rebasing.html, you'll save yourself a lot of energy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Betree I have made the changes. And thanks for the resource that you shared.

@Betree
Copy link
Member

Betree commented Jan 9, 2024

I haven't tried locally, but this crashes the homepage on Vercel: https://opencollective-frontend-git-fork-mayasati-72655d-opencollective.vercel.app.

[Error: EMFILE: too many open files, open '/var/task/node_modules/@icons-pack/react-simple-icons/icons/SiGooglemaps.mjs'] {
  errno: -24,
  code: 'EMFILE',
  syscall: 'open',
  path: '/var/task/node_modules/@icons-pack/react-simple-icons/icons/SiGooglemaps.mjs',
  page: '/home'
}
Error: Runtime exited with error: exit status 1
Runtime.ExitError

@MayaSatishRao
Copy link
Contributor Author

MayaSatishRao commented Jan 9, 2024

@Betree I found this phosphor-icons/react#45 (comment) and tried to make the changes accordingly. It was working fine locally but can you check if this resolves the error

@Betree
Copy link
Member

Betree commented Jan 17, 2024

@Betree I found this phosphor-icons/react#45 (comment) and tried to make the changes accordingly. It was working fine locally but can you check if this resolves the error

Still getting the same issue

image

@MayaSatishRao
Copy link
Contributor Author

@Betree what should I do to remove this error? Should I use a different library instead of react-simple-icons?

@Betree
Copy link
Member

Betree commented Jan 17, 2024

@Betree what should I do to remove this error?

I have no clue on how to resolve this, and the resolution of this issue is not a priority so I can't invest time in researching solutions.

If you don't have the time to research it yourself, I suggest closing the PR and moving on to something more straightforward.

@MayaSatishRao
Copy link
Contributor Author

@Betree can I use react-icons https://www.npmjs.com/package/react-icons instead of react-simple-icons?

@Betree
Copy link
Member

Betree commented Jan 29, 2024

I don't have the bandwidth to research and decide on an icon library. I suggest closing the PR if we can't make it work with the initial specs.

@MayaSatishRao
Copy link
Contributor Author

@Betree I will look again for a solution. Please do not close this issue till then.

@MayaSatishRao
Copy link
Contributor Author

@Betree Can you check it now?

Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

This seems to work; thanks for iterating @MayaSatishRao!

The last thing we must do is rebase the PR on main to solve the conflicts.

@MayaSatishRao
Copy link
Contributor Author

@Betree Do I need to perform rebase to solve conflict?

@Betree
Copy link
Member

Betree commented Jan 30, 2024

If you're not comfortable with it, I can take care of that part

@MayaSatishRao
Copy link
Contributor Author

Can you please do the rebase part? I'm not sure on how to do this

Copy link
Member

@Betree Betree left a comment

Choose a reason for hiding this comment

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

@MayaSatishRao Your fix doesn't seem to work with the latest version of @icons-pack/react-simple-icons

@MayaSatishRao
Copy link
Contributor Author

MayaSatishRao commented Jan 30, 2024

I updated react-simple-icons package to latest version and checked. It is working fine locally. But in the editor I am getting the error.
Screenshot 2024-01-30 190903

@Betree
Copy link
Member

Betree commented Nov 19, 2024

Resolved in #10810

@Betree Betree closed this Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Old Twitter logo is in OpenCollective page
2 participants