Fix visual map issues #17074

christophwen · 2023-06-28T14:16:17Z

hover background color of zoom control in dark mode
map light mode when dark theme is used
background color of zoom control with map dark mode when light theme is used

Breaking change

Proposed change

This PR contains multiple fixes for different visual issues with the map (card).

The hover background color of the maps zoom control when the map is in dark mode is way to white
When setting the map to light mode when dark theme is used, the map isn't shown in light mode (it is dark mode but with a bit more contrast)
The background color of the maps zoom control with the map in dark mode when light theme is used is white (should be dark, so that the icons are visible)

This summery shows how the map's look with all possible configurations:

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 too dark map issue when using dark theme #15587, Map card, new contrasted design: the "the dark_mode" option does not work #13333 (maybe, not sure)
This PR is related to issue or discussion: Change map styles to "Voyager" #13227, Map card, new contrasted design: the "the dark_mode" option does not work #13333
Link to documentation pull request: Replace map card dark_mode with theme_mode home-assistant.io#32352

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:

Documentation added/updated for www.home-assistant.io

bramkragten · 2023-06-28T14:22:02Z

src/panels/lovelace/cards/types.ts

@@ -286,7 +286,7 @@ export interface MapCardConfig extends LovelaceCardConfig {
  entities?: Array<EntityConfig | string>;
  hours_to_show?: number;
  geo_location_sources?: string[];
-  dark_mode?: boolean;


This is a breaking change, if we are going to change this, we should at least make it backwards compatible

Sorry for that, I will fix this.

christophwen · 2023-06-28T15:17:37Z

I forgot this image which shows the new Theme Mode selection in the map editor.

karwosts · 2023-06-28T15:57:59Z

Does that radio button set need a title?

christophwen · 2023-06-28T16:00:15Z

Does that radio button set need a title?

Good questions, I didn't thought about that. I just assumed that it is clear. I will think about that and maybe I get more opinions on that.

christophwen · 2023-06-28T17:24:34Z

I got some feedback about the radio buttons that a title is preferred. (11/11 people)
I also got the suggestion to use a dropdown for this.
@karwosts and @bramkragten what are your opinions about the dropdown idea?

karwosts · 2023-06-28T17:42:02Z

I will point out the default behavior of selector-select is radio buttons with 5 or less options, and dropdown with 6 or more.

Personally I have no opinion between the two, but if the design decision was previously made that selectors with less than 6 items use radio buttons, I see no reason to override that, might as well stay consistent?

Note to get the title I think all you need to do is provide a translation key for "theme_mode", but you've already used that as a group name for the modes. Maybe just rename "theme_mode" : { ... } -> "theme_modes" : { ... }, and then you can add "theme_mode" : "Theme Mode"

christophwen · 2023-06-28T17:50:35Z

I will point out the default behavior of selector-select is radio buttons with 5 or less options, and dropdown with 6 or more.

Personally I have no opinion between the two, but if the design decision was previously made that selectors with less than 6 items use radio buttons, I see no reason to override that, might as well stay consistent?

I see no reason too. In the user's profile the dropdown is used multiple times with less than 6 options.
In the M3 Design Guide I found out that if saving space is important a dropdown is a good alternative. (This applies kinda to this issue here)

Note to get the title I think all you need to do is provide a translation key for "theme_mode", but you've already used that as a group name for the modes. Maybe just rename "theme_mode" : { ... } -> "theme_modes" : { ... }, and then you can add "theme_mode" : "Theme Mode"

Thank you!

siw1973 · 2023-07-04T12:13:46Z

@christophwen @bramkragten Thanks for working on this, it looks like this has had all the checks passed.

I don't know the process but will this now make the next release ?

bramkragten · 2023-07-04T12:26:50Z

I don't know the process but will this now make the next release ?

No, the beta for the next release (tomorrow) started last Wednesday. We don't add new features after the beta has started, only bug fixes.

This will be in the release of August.

christophwen · 2023-07-05T12:45:59Z

Changed ratio button selection of theme mode to dropdown and reorganized the editor view a little bit.
(Fixed unpleasant horizontal scroll view, see screenshots above)

siw1973 · 2023-07-05T13:12:01Z

I don't know the process but will this now make the next release ?

No, the beta for the next release (tomorrow) started last Wednesday. We don't add new features after the beta has started, only bug fixes.

This will be in the release of August.

Thanks for the update 👍

bramkragten · 2023-07-13T14:27:24Z

src/components/map/ha-map.ts

+      #map.light {
+        background: #ffffff;
+        color: #000000;


I think we should only use hard coded colors when the map darkmode setting differs from the theme darkmode. Otherwise we should just use theme colors

The #000000 is needed to show the icon of the home (house) in black, when HA is in dark mode and map should use the light theme. I will change it to --white-color. (#ffffff → --black-color)

We should add a force flag to ha-map, that only hardcodes colors when the mode doesnt match the theme hass.themes.darkMode.

bramkragten · 2023-07-13T14:28:23Z

src/components/map/ha-map.ts

+      #map.light {
+        background: #ffffff;
+        color: #000000;
+        --map-filter: invert(0);


What does this do? --map-filter: unset;

This forces the map to appear in light mode when HA is in dark mode. With --map-filter: unset; the map stays in dark mode.

bramkragten · 2023-07-13T14:28:44Z

src/components/map/ha-map.ts

      .leaflet-tile-pane {
        filter: var(--map-filter);
      }
      .dark .leaflet-bar a {
-        background-color: var(--card-background-color, #1c1c1c);
+        background-color: #1c1c1c;


Same, use theme if possible

This is needed to show the + and - of the zoom control in black, when HA is in light theme and the map in dark mode.

bramkragten · 2023-07-13T14:31:25Z

src/panels/lovelace/editor/config-elements/hui-map-card-editor.ts

@@ -94,33 +125,32 @@ export class HuiMapCardEditor extends LitElement implements LovelaceCardEditor {
      <ha-form
        .hass=${this.hass}
        .data=${this._config}


We should migrate the old config (dark_mode) to the new config so it is displayed correctly in the form, and is saved in the new format

It would be great if we can discuss this on Discord, because right now, I am not sure what I have to do here.

bramkragten · 2023-07-13T14:32:15Z

Please open a documentation PR for these changes.

christophwen · 2023-07-18T18:23:26Z

Please open a documentation PR for these changes.

Could you please explain to me what a documentation PR is, I have never created one before. Then I will do that :)

karwosts · 2023-07-19T04:50:43Z

Could you please explain to me what a documentation PR is, I have never created one before. Then I will do that :)

If we're changing the options for map card, go here:

https://www.home-assistant.io/dashboards/map/

Go down to the bottom of the page, click the Edit button. It will take you to github page with the markdown for that webpage. Click the edit button, make changes detailing the new configuration options, and submit them as a PR to the next branch. Once the PR is created, add a link back here.

christophwen · 2023-07-26T16:09:58Z

Short Update: I will continue as soon as I get some answers. Then I will finish the PR and create the documentation PR as follow-up.

christophwen · 2023-09-04T21:19:22Z

Hey, I was on vacation, but now I am back. I will continue soon with this PR.

smithbill17 · 2023-09-27T04:47:32Z

Is this still progressing?

The original issue #15587 had been marked as stale but the problem is still present in Home Assistant 2023.9.3 & Companion App 2023.9.2-full

siw1973 · 2023-10-21T12:24:25Z

Short Update: I will continue as soon as I get some answers. Then I will finish the PR and create the documentation PR as follow-up.

@christophwen did you get the feedback you needed to raise the PR?

I know a few people are waiting for and appreciate the work you have put in so far.

christophwen · 2023-10-21T16:56:05Z

@siw1973 Sorry, I got carried away by something else. I will do my best to finish the PR.

bramkragten · 2023-11-16T11:13:20Z

Check #18667, it added a class for forced dark that you should use in this PR too.

smithbill17 · 2023-11-16T11:39:58Z

Aren't we adding a 'Lite' mode map, even when the Home Assistant theme is set to 'Dark' mode? (basically so the map can actually be seen clearly)

redam · 2023-11-16T12:07:04Z

yes we need a way to force "light" mode, not dark mode

bramkragten · 2023-11-16T17:12:48Z

We need both.

- hover background color of zoom control in dark mode - map light mode when dark theme is used - background color of zoom control with map dark mode when light theme is used

- Additionally fixed unpleasant horizontal scrollbar in map editor

christophwen · 2023-11-16T18:07:07Z

Check #18667, it added a class for forced dark that you should use in this PR too.

I rebased and added the class. (+ 'force-light')

bramkragten · 2023-11-18T16:34:17Z

src/components/map/ha-map.ts

@@ -159,6 +159,7 @@ export class HaMap extends ReactiveElement {
    const map = this.shadowRoot!.getElementById("map");
    map!.classList.toggle("dark", darkMode);
    map!.classList.toggle("forced-dark", this.darkMode);
+    map!.classList.toggle("foced-light", !darkMode);


forced-light should only be set when this.darkMode === false otherwise it will also add the forced class when then theme is actually light mode. forced here means the map dark mode is different than the dark mode of the theme.

ildar170975 · 2023-12-09T17:58:10Z

Imho the dark_mode option should (kind of a FR):
– force the map to be dark - if TRUE
– force the map to be light - if FALSE
– force the map to follow a current theme - if not defined.

github-actions · 2024-03-08T18:06:00Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

siw1973 · 2024-03-14T10:37:11Z

This is a real shame as this has had some work done on it, but seems to have been bogged down by a lack of understanding on process and clarity as to what the next steps need to be.

If this could be resurected and dragged over the line then it would be really helpful to a few people linked to this issue and also I expect many other people who just don't use GitHub.

karwosts · 2024-04-17T00:37:09Z

@christophwen - If you have no intention of returning to this in the near future, I will push some changes that I think can bring this to completion.

If you don't want me to make edits to your branch, please state your objection, otherwise I will do so in a few days.

christophwen · 2024-04-17T05:07:02Z

@karwosts it would be great if you make changes to get this done. My free time is very limited right now and I am not the best with CSS changes. Thank you :)

karwosts · 2024-04-17T13:15:28Z

Hmm I thought I would be able to push to this from the CLI, but I'm getting 403 authorization problems whatever I try to do.

Pushing to https://github.com/christophwen/home-assistant-frontend
remote: Permission to christophwen/home-assistant-frontend.git denied to karwosts.
fatal: unable to access 'https://github.com/christophwen/home-assistant-frontend/': The requested URL returned error: 403

(Maybe I should have cloned the upstream PR branch instead of your repo's branch? not exactly sure)

Unless I get that figured out I just spawned a new PR at #20541 with these changes and fixes to get around the 403 problem.

bramkragten · 2024-04-22T07:43:08Z

#20541

karwosts · 2024-04-24T13:17:49Z

The duplicate PR was merged. Thanks @christophwen

christophwen · 2024-04-24T14:36:06Z

@karwosts thanks for finishing!

home-assistant bot added the cla-signed label Jun 28, 2023

bramkragten reviewed Jun 28, 2023

View reviewed changes

christophwen requested a review from bramkragten June 28, 2023 15:18

bramkragten reviewed Jul 13, 2023

View reviewed changes

bramkragten added the Documentation Needed label Jul 13, 2023

This comment was marked as off-topic.

Sign in to view

github-actions bot removed the Design Related to Home Assistant design gallery label Sep 26, 2023

christophwen added 3 commits November 16, 2023 19:03

Fix visual map issues (home-assistant#15587)

1c989f2

- hover background color of zoom control in dark mode - map light mode when dark theme is used - background color of zoom control with map dark mode when light theme is used

Fix breaking change (home-assistant#15587)

9aafff7

Change theme mode selection to use dropdown (home-assistant#15587)

c45b621

- Additionally fixed unpleasant horizontal scrollbar in map editor

christophwen force-pushed the visual-map-issues branch from 772ef8a to c45b621 Compare November 16, 2023 18:04

bramkragten reviewed Nov 18, 2023

View reviewed changes

karwosts mentioned this pull request Jan 29, 2024

Fix map icon color #19567

Merged

9 tasks

github-actions bot added the stale label Mar 8, 2024

github-actions bot removed the stale label Mar 14, 2024

karwosts mentioned this pull request Apr 17, 2024

Fix visual map issues #20541

Merged

9 tasks

karwosts removed the Documentation Needed label Apr 17, 2024

silamon changed the title ~~Fix visual map issues (#15587)~~ Fix visual map issues Apr 19, 2024

bramkragten closed this Apr 22, 2024

Fix visual map issues #17074

Fix visual map issues #17074

Conversation

christophwen commented Jun 28, 2023 • edited by karwosts Loading

Breaking change

Proposed change

This PR contains multiple fixes for different visual issues with the map (card).

Type of change

Example configuration

Additional information

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christophwen commented Jun 28, 2023

karwosts commented Jun 28, 2023

christophwen commented Jun 28, 2023

christophwen commented Jun 28, 2023

karwosts commented Jun 28, 2023

christophwen commented Jun 28, 2023

siw1973 commented Jul 4, 2023 • edited Loading

bramkragten commented Jul 4, 2023

christophwen commented Jul 5, 2023

siw1973 commented Jul 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bramkragten commented Jul 13, 2023

christophwen commented Jul 18, 2023

karwosts commented Jul 19, 2023

christophwen commented Jul 26, 2023

christophwen commented Sep 4, 2023

This comment was marked as off-topic.

smithbill17 commented Sep 27, 2023

siw1973 commented Oct 21, 2023

christophwen commented Oct 21, 2023

bramkragten commented Nov 16, 2023

smithbill17 commented Nov 16, 2023

redam commented Nov 16, 2023

bramkragten commented Nov 16, 2023

christophwen commented Nov 16, 2023

Choose a reason for hiding this comment

ildar170975 commented Dec 9, 2023

github-actions bot commented Mar 8, 2024

siw1973 commented Mar 14, 2024

karwosts commented Apr 17, 2024

christophwen commented Apr 17, 2024

karwosts commented Apr 17, 2024 • edited Loading

bramkragten commented Apr 22, 2024

karwosts commented Apr 24, 2024

christophwen commented Apr 24, 2024

christophwen commented Jun 28, 2023 •

edited by karwosts

Loading

siw1973 commented Jul 4, 2023 •

edited

Loading

karwosts commented Apr 17, 2024 •

edited

Loading