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 clickForMoreInfo to statistics graph card #19178

Conversation

mib1185
Copy link
Contributor

@mib1185 mib1185 commented Dec 29, 2023

Proposed change

This is adopts #19606 from history charts for statistics chart.

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:

@karwosts
Copy link
Contributor

karwosts commented Dec 29, 2023

I think this feature is problematic for mobile users (they can't activate tooltips anymore), and the original feature is undergoing some revision (#19144). Might make sense to wait for that to play out before expanding this wider.

@mib1185
Copy link
Contributor Author

mib1185 commented Dec 29, 2023

#19144 nice one 👍 i agree, we should wait for this

@mib1185 mib1185 marked this pull request as draft December 29, 2023 14:07
@karwosts
Copy link
Contributor

Looks like we've gone ahead with making the more-info popups a mouse feature only, so if you wanted to replicate the same logic from #19606, that could probably go ahead now.

@mib1185 mib1185 force-pushed the statistics-graph-card/add-click-for-more-information branch from daab881 to 1460d71 Compare February 11, 2024 18:59
@mib1185
Copy link
Contributor Author

mib1185 commented Feb 11, 2024

changes from #19606 adopted

@mib1185 mib1185 marked this pull request as ready for review February 11, 2024 19:00
@mib1185 mib1185 requested a review from karwosts February 11, 2024 21:21
@mib1185
Copy link
Contributor Author

mib1185 commented Feb 28, 2024

Hi @karwosts do you see a chance that we could get this in before beta cut?

@karwosts
Copy link
Contributor

karwosts commented Feb 28, 2024

I don't really do merging, I let the maintainers handle that. But it looks like the first beta cut for frontend has just gone out 😢

@bramkragten
Copy link
Member

bramkragten commented Feb 29, 2024

This will not be merged before we have consensus about the button logic for mobile, it could be merged if it would use the same logic as the current history graph does.

@mib1185
Copy link
Contributor Author

mib1185 commented Feb 29, 2024

it should be the same, as it adopts #19606

@mib1185 mib1185 requested a review from bramkragten February 29, 2024 18:29
@mib1185 mib1185 requested a review from bramkragten March 4, 2024 17:22
@bramkragten bramkragten merged commit bf02891 into home-assistant:dev Mar 5, 2024
13 checks passed
@mib1185 mib1185 deleted the statistics-graph-card/add-click-for-more-information branch March 5, 2024 15:12
@mib1185
Copy link
Contributor Author

mib1185 commented Mar 5, 2024

is it to late to get it in current beta? 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants