nielsfaber / scheduler-card

HA Lovelace card for control of scheduler entities
GNU General Public License v3.0
926 stars 117 forks source link

Add the time domain (time entities) to the list of standard configuration actions #808

Open pdcastro opened 8 months ago

pdcastro commented 8 months ago

This PR addresses issue #807. Please check that issue’s description for background information (use cases, justification).

Screenshots of what this PR achieves

Scheduler Card Configuration — Included Entities Before this PR, time entities were not listed — there wasn’t a ‘time’ row in the screenshot below. After this PR, the ‘time’ row is listed, allowing time entities to be selected.

Scheduler Card Configuration - Entities - Time

New Schedule - Entity Editor - Time Entity

New Schedule - Entity Editor

New Schedule - Time Editor - Time Entity - Set Value

New Schedule - Time Editor

New Schedule - Time Editor - Time Entity - Make Scheme

New Schedule - Time Editor - Scheme

Scheduler Card - Configured Entities - Time Entity

Scheduler Card - Configured Entities
nielsfaber commented 8 months ago

I'm impressed with your efforts, the PR looks pretty good to me. Sadly, the fact that you had to touch 20+ files to implement this feature only emphasises that the code is in need of optimisation. As said, I am currently working on a rewrite of the code. For this reason I think we have to park this PR for a while (I am not adding new features to the current code base anymore). I am hopeful that it can be 'ported' to the new version without too much extra effort. The new version should be 'feature-complete' within the next month, we can continue from there. A general comment (for future contribution): I think it's helpful to open a feature request to discuss/align on a new feature before starting on the implementation. This reduces the risk of writing code that may not be adopted in the project. My apologies for putting your contribution on hold.

pdcastro commented 8 months ago

[…] For this reason I think we have to park this PR for a while […]

That’s all right. I take comfort in knowing that you seem happy to incorporate this feature in principle. I had originally planned to discuss it before any implementation, 😇 and I thought it should include “photoshopping” what the time editor screen might look like. But then I thought: “Why don’t just do a quick hack of the time editor screen on its own, just to load a <ha-time-input> component there?” “Would that even be possible?” “Do custom cards have the ability of instantiating just any HA frontend component?” ”How would I even go about hacking it?” I actually had so many questions as I had never worked on a frontend project before, and I was really curious. So I just started playing with the code a bit, not planning on any large implementation, but each small step led to more questions and further small steps to answer those questions... “Now I’ve come this far, might as well finish it!” I had no idea that the final solution would involve touching so many files, honestly I initially thought I’d have to change “just a few HTML templates” — because that’s what a custom card is, right? 😄 How hard could that possibly be? 😅

By the way, I now appreciate so much more the effort that you put in creating this custom card. There is so much logic in it that end users are just oblivious to. On behalf of clueless end users, thank you! 🙏

Meanwhile, I am already using this feature through my fork of your repo, so in any event I am already benefiting from it. 🙂

github-actions[bot] commented 7 months ago

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

pdcastro commented 7 months ago

Rebased on the main branch in order to remove the ‘stale’ label and to fix merge conflicts.

github-actions[bot] commented 6 months ago

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

github-actions[bot] commented 5 months ago

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

github-actions[bot] commented 4 months ago

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

github-actions[bot] commented 3 months ago

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

github-actions[bot] commented 2 months ago

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

github-actions[bot] commented 1 month ago

This pull request is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

pdcastro commented 1 month ago

Bump