Skip to content

Add toggl_track #5023

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 28, 2023
Merged

Add toggl_track #5023

merged 3 commits into from
Dec 28, 2023

Conversation

kquinsland
Copy link
Contributor

Proposed change

This is logo/icon for a custom component i'm working on / wrapping up.
The company provides a press kit which includes svg which I scaled, converted and then crushed.

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

  • This PR fixes or closes issue: fixes #
  • Link to code base pull request:
  • Link to documentation pull request:
  • Link to integration documentation on our website:

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)

kquinsland and others added 2 commits December 24, 2023 21:17
@kquinsland
Add toggl_track
77f9217 
light/dark versions of logo and icon and @2x.png.
@kquinsland
oops, omit logo.svg
0824a03 
Had a bug in my `for f in ... git add` :)
@kquinsland
Copy link
Contributor Author

Accidentally had logo.svg, that's now removed.
Other validation warnings are related to files that are already in the repo :(

frenck
frenck requested changes Dec 27, 2023
View reviewed changes
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.

Hi there @kquinsland 👋

I could not find the public repository providing this custom integration. Could you point me to it?

Thanks! 👍

../Frenck

@home-assistant home-assistant bot marked this pull request as draft December 27, 2023 09:27
@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

kquinsland added a commit to kquinsland/ha-toggl-track that referenced this pull request Dec 27, 2023
@kquinsland
Initial commit for the HA custom component that uses lib-toggl c
104f08b 
Docs are in good enough shape to release.
Code works but is far from perfect :D.

the pydantic/lib-toggle issue is a blocker for general use. Issues opened and linked in docs.

Pushing this and making repo public for: home-assistant/brands#5023 (review)
@kquinsland
Copy link
Contributor Author

Hi there @kquinsland 👋

I could not find the public repository providing this custom integration. Could you point me to it?

Sure thing. It's now public, lives here: https://github.com/kquinsland/ha-toggl-track

I was hoping to release yesterday but got hit with pydantic v2 / home-assistant/core#99218.

@frenck frenck marked this pull request as ready for review December 27, 2023 21:12
@home-assistant home-assistant bot requested a review from frenck December 27, 2023 21:12
@frenck frenck marked this pull request as draft December 27, 2023 21:13
@frenck
Copy link
Member

frenck commented Dec 27, 2023

Sure thing. It's now public, lives here: kquinsland/ha-toggl-track

That link doesn't work.

@kquinsland
Copy link
Contributor Author

Sure thing. It's now public, lives here: kquinsland/ha-toggl-track

That link doesn't work.

Give it a try now :). Github now makes you triple confirm you want to make a repo public

@kquinsland kquinsland marked this pull request as ready for review December 28, 2023 19:56
frenck
frenck approved these changes Dec 28, 2023
View reviewed changes
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, @kquinsland 👍

../Frenck

@frenck frenck merged commit 63217f2 into home-assistant:master Dec 28, 2023
@kquinsland kquinsland deleted the feat/add-toggl-track branch December 28, 2023 21:49
@kquinsland
Copy link
Contributor Author

Thanks, @kquinsland 👍

Just for my curiosity, can you explain why custom components need to use icons on the CDN?

I think this pattern is 100% the right move for official/core integrations but was wondering why custom components cant serve icons out of their root directory.

I didn't find any explicit rationale and was just curious if there was a reason beyond "writing two code paths for fetching images is just not something we want to spend time on".

Anyways, thanks and happy new year! (Looking forward to the 24.01 release :D)

@frenck
Copy link
Member

frenck commented Dec 28, 2023

Just for my curiosity, can you explain why custom components need to use icons on the CDN?

There are multiple reasons. For example:

  • It gives us a small piece of control in upstream branding (and ensures we don't violate any branding).
  • We will be able to handle requests from brands that don't want us to use their brand
  • We can have a small peek into the custom integration upstream (we often catch dependency issues, domain issues, and other common problems).
  • We use this as a list of known/common domains for:
    • Analytics data (opt-in only)
    • Signal domain collisions

There have been multiple ideas on how to handle branding, including shipping them with core (increases distribution size drastically), publishing an add-on that pulls them locally, caching them locally, and... many others.

Up until this time, no concrete plans, architectural proposals, or suggestions of contributions to any of them have been made. 🤷

../Frenck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frenck frenck frenck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kquinsland @frenck