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 period selection to energy dashboard #9756

Merged
merged 6 commits into from
Aug 10, 2021
Merged

Add period selection to energy dashboard #9756

merged 6 commits into from
Aug 10, 2021

Conversation

bramkragten
Copy link
Member

@bramkragten bramkragten commented Aug 9, 2021

Proposed change

Add week, month, year views to energy dashboard.

image

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue or discussion:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@@ -82,6 +177,15 @@ export class HuiEnergyPeriodSelector extends SubscribeMixin(LitElement) {
private _updateDates(energyData: EnergyData): void {
this._startDate = energyData.start;
this._endDate = energyData.end || endOfToday();
this._period = isSameDay(this._startDate, this._endDate)
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should store "period" in the energy data so it's easier to show such buttons etc.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder even if we should store just a "start date" that combined with period = start + end.

That way we can still do -1 week or -1 month but, scaling up/down will keep us in the same 'range'

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to keep energy and all other elements as flexible as possible really, so it could handle any date range. Just the period selector is the part that is limiting now... I agree that this part is ugly, so maybe we could save the period in energy data just for this... 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

Made this logic a bit simpler now...

@balloob
Copy link
Member

balloob commented Aug 10, 2021

For a future PR… but can we hide the 12PMs?

image

balloob
balloob previously approved these changes Aug 10, 2021
@bramkragten bramkragten enabled auto-merge (squash) August 10, 2021 22:17
@bramkragten bramkragten merged commit dc50e54 into dev Aug 10, 2021
@delete-merged-branch delete-merged-branch bot deleted the period-selection branch August 10, 2021 23:22
@github-actions github-actions bot locked and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants