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

Add EufyLife logo #4039

Merged
merged 3 commits into from
Jan 17, 2023
Merged

Add EufyLife logo #4039

merged 3 commits into from
Jan 17, 2023

Conversation

bdr99
Copy link
Contributor

@bdr99 bdr99 commented Jan 15, 2023

Proposed change

Add logo for the proposed EufyLife integration.

Also add logo and icon for eufy brand.

Type of change

  • Add a new logo or icon for a new core integration
  • Add a missing icon or logo for an existing core integration
  • Add a new logo or icon for a custom integration (custom component)
  • Replace an existing icon or logo with a higher quality version
  • Removing an icon or logo

Additional information

Checklist

  • The added/replaced image(s) are PNG
  • Icon image size is 256x256px (icon.png)
  • hDPI icon image size is 512x512px for (icon@2x.png)
  • Logo image size has min 128px, but max 256px, on the shortest side (logo.png)
  • hDPI logo image size has min 256px, but max 512px, on the shortest side (logo@2x.png)

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

The icons provided seems to be application icons from their apps?
Please use the eufy branding instead.

Please note, the use of more specific product branding is allowed, but that would only be accepted if a brand definition in the Core repository is made under which this integration would be put.

../Frenck

@frenck frenck added has-parent This PR has a parent PR in a other repo in-progress This PR/Issue is currently being worked on labels Jan 15, 2023
@bdr99
Copy link
Contributor Author

bdr99 commented Jan 15, 2023

@frenck I was actually planning to create a brand definition in core, since there will be two Eufy integrations. The existing eufy integration integrates devices from sold under EufyHome product line, and the new eufylife_ble integration will cover the EufyLife product line. But I was going to wait to create the brand definition core PR until after the integration PR is merged. Would it be better if I create it now? Or alternatively, should I add the brand to my existing core PR for the integration?

@frenck
Copy link
Member

frenck commented Jan 16, 2023

I was actually planning to create a brand definition in core, since there will be two Eufy integrations.

Great, but this PR and the core PR don't reflect that.

But I was going to wait to create the brand definition core PR until after the integration PR is merged. Would it be better if I create it now?

Can be done in these PRs directly.

../Frenck

@bdr99
Copy link
Contributor Author

bdr99 commented Jan 16, 2023

@frenck OK, I updated this PR and the core PR to add the eufy brand.

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

The core_integrations/eufy folder can be symlinked to the brands one

@bdr99
Copy link
Contributor Author

bdr99 commented Jan 16, 2023

@frenck If I did that, then the eufy integration would no longer be able to have its own icon.

@bdr99
Copy link
Contributor Author

bdr99 commented Jan 16, 2023

To clarify the above statement, the current icon used by the eufy integration is the logo for the EufyHome product line/app, which makes sense because those are the products that are compatible with that integration. Similarly, the eufylife_ble integration will have the icon for the EufyLife product line. I actually think the Eufy integration should be renamed to EufyHome, and am planning to create a PR for this after home-assistant/core#85907 is merged.

@bdraco bdraco added the parent-merged The parent PR has been merged already label Jan 17, 2023
@frenck
Copy link
Member

frenck commented Jan 17, 2023

@frenck If I did that, then the eufy integration would no longer be able to have its own icon.

Thanks, that is clear! 👍

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Thanks, @bdr99 👍

../Frenck

@frenck frenck merged commit f577ddf into home-assistant:master Jan 17, 2023
tsunglung pushed a commit to tsunglung/brands that referenced this pull request Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-parent This PR has a parent PR in a other repo in-progress This PR/Issue is currently being worked on parent-merged The parent PR has been merged already
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants