Skip to content

More Info: Add History Tab#6758

Merged
zsarnett merged 8 commits intodevfrom
more-info-history
Sep 3, 2020
Merged

More Info: Add History Tab#6758
zsarnett merged 8 commits intodevfrom
more-info-history

Conversation

@zsarnett
Copy link
Contributor

@zsarnett zsarnett commented Sep 1, 2020

Proposed change

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

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:

@bramkragten
Copy link
Member

For the layout, can we put the time above the line for the logbook?

@zsarnett
Copy link
Contributor Author

zsarnett commented Sep 1, 2020

Yea I can probably add a narrow layout to the log book

const yesterday = new Date(new Date().getTime() - 24 * 60 * 60 * 1000);
const now = new Date();
const [entries] = await Promise.all([
getLogbookData(
Copy link
Member

@bdraco bdraco Sep 1, 2020

Choose a reason for hiding this comment

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

We will need to teach getLogbookData to pass the entity_matches_only (only in this case) to use home-assistant/core#39555

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Thanks @bdraco

@zsarnett
Copy link
Contributor Author

zsarnett commented Sep 1, 2020

image

@bdraco
Copy link
Member

bdraco commented Sep 1, 2020

Also no harm in merging this before the backend is done as it will just be ~2x slower until the backend change is done.

Co-authored-by: J. Nick Koston <nick@koston.org>
@bdraco
Copy link
Member

bdraco commented Sep 2, 2020

Screen_Shot_2020-09-02_at_1_32_01_PM

Maybe suppress the name since we are already in a modal that is only about the name?

@zsarnett
Copy link
Contributor Author

zsarnett commented Sep 2, 2020

image


private async _refreshData(): Promise<void> {
this._isLoading = true;
this._fetchPersonNames();
Copy link
Member

Choose a reason for hiding this comment

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

We only need to do this when person entities changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do we know they changed?

Copy link
Member

Choose a reason for hiding this comment

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

Check for state changes, but might not be worth the trouble, never mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I just fetch it once on first updated and not worry about it?

@zsarnett zsarnett merged commit 1431e75 into dev Sep 3, 2020
@zsarnett zsarnett deleted the more-info-history branch September 3, 2020 23:28
@bramkragten bramkragten mentioned this pull request Sep 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
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.

4 participants