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

Allow for PNG brand logos in experience block #2953

Closed
wants to merge 2 commits into from
Closed

Allow for PNG brand logos in experience block #2953

wants to merge 2 commits into from

Conversation

rodrigoalcarazdelaosa
Copy link
Contributor

Purpose

Fixes #2952.

Documentation

This documentation should be updated to reflect the change.

@netlify
Copy link

netlify bot commented May 31, 2023

Deploy Preview for markdown-slides canceled.

Name Link
🔨 Latest commit 3a9d21d
🔍 Latest deploy log https://app.netlify.com/sites/markdown-slides/deploys/65266071069f0d000838bdd0

@netlify
Copy link

netlify bot commented May 31, 2023

Deploy Preview for hugo-portfolio-theme canceled.

Name Link
🔨 Latest commit 3a9d21d
🔍 Latest deploy log https://app.netlify.com/sites/hugo-portfolio-theme/deploys/65266071252fef0008a87cf2

@netlify
Copy link

netlify bot commented May 31, 2023

Deploy Preview for academic-demo canceled.

Name Link
🔨 Latest commit 3a9d21d
🔍 Latest deploy log https://app.netlify.com/sites/academic-demo/deploys/652660712a2c260009c52074

@etnguyen03
Copy link

If this gets merged/approved, I'd be interested in having something similar with the features block as well (and can open a PR if necessary)

@github-actions
Copy link

This PR is stale because it has not had any recent activity. The resources of the project maintainers are limited, and so we are asking for your help.

If you feel that the PR is still relevant in the latest release, consider making the PR easier to review and finding developers to help review the PR.

Please be mindful that although we encourage PRs, we cannot expand the scope of the project in every possible direction. There will be requests that don't make the roadmap.

This PR will automatically close soon if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jul 31, 2023
@rodrigoalcarazdelaosa
Copy link
Contributor Author

Since the triggering issue was closed, does it make sense to keep this PR?

@github-actions github-actions bot removed the stale label Aug 1, 2023
@gcushen
Copy link
Collaborator

gcushen commented Aug 2, 2023

Since the triggering issue was closed, does it make sense to keep this PR?

Has this PR been tested? I recall a message in Discord (maybe a month ago) commenting that PNG logos were getting stretched or not appearing as expected - was that related to this PR?

@rodrigoalcarazdelaosa
Copy link
Contributor Author

Can you test it with your use case, @rabinadk1? I think those issues with logos were related to the accomplishments block, @gcushen

@rabinadk1
Copy link

Can you test it with your use case, @rabinadk1? I think those issues with logos were related to the accomplishments block, @gcushen

Works for me. Thanks.

@github-actions
Copy link

This PR is stale because it has not had any recent activity. The resources of the project maintainers are limited, and so we are asking for your help.

If you feel that the PR is still relevant in the latest release, consider making the PR easier to review and finding developers to help review the PR.

Please be mindful that although we encourage PRs, we cannot expand the scope of the project in every possible direction. There will be requests that don't make the roadmap.

This PR will automatically close soon if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 20, 2023
@rodrigoalcarazdelaosa
Copy link
Contributor Author

Can you test it with your use case, @rabinadk1? I think those issues with logos were related to the accomplishments block, @gcushen

Works for me. Thanks.

@gcushen

@github-actions github-actions bot removed the stale label Sep 21, 2023
@gcushen
Copy link
Collaborator

gcushen commented Oct 5, 2023

a few thoughts:

  • you've removed the file extension from the error message, so it could be unclear to the user exactly what is missing
  • we should not be adding new features to the deprecated v1 blocks, only to the current set of blocks
  • if we add PNG support for the Experience block, users will likely expect PNG is supported in the Accomplishments block, so if we change one of these blocks, we should probably change both for consistency
  • we should try to read the file as an SVG first as the icon library folder is intended to be an SVG icon library, and then perhaps fall back to searching for a PNG if it can't find SVG
  • there are also now Git conflicts in the PR which would need addressing

@rodrigoalcarazdelaosa
Copy link
Contributor Author

Closing the PR as this comment already offers a solution.

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.

Can we allow the logos to be png?
4 participants