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

Don't hide location entities that are "home" in the MapViewStrategy #23462

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Hypfer
Copy link
Contributor

@Hypfer Hypfer commented Dec 26, 2024

Proposed change

I've discovered that the MapViewStrategy that populates the default map dashboard filters any entity that is "home" no matter if it has geolocation data. Checking the history, I can see that this is how it always worked:
https://github.com/home-assistant/frontend/blame/7542c03dbbfd64e20dbc061ab6d271d4aec8a76a/src/panels/map/ha-panel-map.ts#L61-L87

I'd like to challenge that, because to me, it is very confusing.

I first thought that maybe my android companion app was misconfigured, because even though I enabled my location, I just couldn't see anything. There was simply no feedback excluding maybe the devtools where my person entity suddenly gained latitude/longitude attributes.
I was considering that maybe I'm supposed to "Take Control" of the Map dashboard, however that also seemed wrong so I started digging and eventually found that filter.

Another inconsistency about this filter is that the person marker will show up while in any other configured zone (e.g. work, school, etc.) but not while home.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue) - arguably a UX bugfix but might also just be an opinion
  • 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

Default config

Additional information

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

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:

@MindFreeze MindFreeze added the Needs UX Pull requests requiring a review from the Home Assistant design team label Dec 27, 2024
@Hypfer
Copy link
Contributor Author

Hypfer commented Dec 30, 2024

This other PR I opened could also be considered related, as - when there are multiple gps and non-gps device_trackers configured - it greatly increases the likelihood that a person entity that is "home" has geo-coordinates in the first place:
home-assistant/core#134075

Probably also something best to be reviewed by the UX Team

@MindFreeze
Copy link
Contributor

We discussed it with the UX team and agreed that this should be an option to the map strategy. Changing it for everyone would be too disruptive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed Needs UX Pull requests requiring a review from the Home Assistant design team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants