-
Notifications
You must be signed in to change notification settings - Fork 5
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
Solve build failing due to connectors null logo #708
base: master
Are you sure you want to change the base?
Solve build failing due to connectors null logo #708
Conversation
@travjenkins Recently the Monday logo was added with this size and format: Is it possible to replace it with the same size of the other connector logos? Just removing the "monday" and keeping the image of the logo would work. |
Visit the preview URL for this PR (updated for commit 793c009): https://estuary-marketing--pr708-brenosalv-bug-707-bu-58rpxwb1.web.app (expires Wed, 12 Mar 2025 01:40:13 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 76f6b095a0752e5d9c6c890267f9fdc3e392161e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const sourceConnectorLogo = sourceConnector.logo | ||
? getImage(sourceConnector.logo?.childImageSharp?.gatsbyImageData) | ||
: null; | ||
const destinationConnectorLogo = destConnector.logo | ||
? getImage(destConnector.logo?.childImageSharp?.gatsbyImageData) | ||
: null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If destConnector.logo?.childImageSharp?.gatsbyImageData
is optionally chained then the ternary should probably just check the full chain to get safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One change needs to happen.
@Brenosalv - no we cannot change the Monday logo. Their rules say it needs the full wide logo. The dashboard had a bug so we pushed a quick change to make the logo smaller while we fix it. Soon it'll be bigger but still wide. |
For the default icon we should use a cloud or something. For Captures (Sources) we use a cloud with arrow pointing up and for materializations (destinations) cloud with arrow pointing down. |
To be clear - this does not solve the problem. The problem is not really due to connector logos being null. The issue is that connector logos that are 100% not null and are in the db are becoming null somehow. This means by always handling null you can have two builds with the same input that end up with different output. This is a big deal as it can cause weird outcomes. We still have to 100% figure out why logos that are there are missing. |
#707
Changes
gatsbyImageData
of null logo property.Tests / Screenshots