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

The gauge card doesn't render correctly on iOS 15.2 / macOS 12.1 in the companion apps #11052

Closed
wants to merge 3 commits into from

Conversation

raducotescu
Copy link
Contributor

Proposed change

  1. enhanced the Safari test from The gauge card doesn't render correctly on Safari 15.1 on macOS / iOS #10766
  2. added special handling for the HA Companion App on macOS/iOS - fixes The gauge card doesn't render correctly on iOS 15.2 / macOS 12.1 in the companion apps #11041

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

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:

…iOS 15.2 / macOS 12.1 in the companion apps

* enhanced the Safari test from home-assistant#10766
* added special handling for the HA Companion App on macOS/iOS
@raducotescu
Copy link
Contributor Author

@balloob, could you please run the workflows for this PR? Who else should review it? I've tested it by running a new HA instance in Docker, to which I mounted the frontend in a volume. Everything worked as expected. Both Safari and the companion apps running on iOS/macOS render the gauge card correctly now.

@raducotescu
Copy link
Contributor Author

All the linked issues mentioned above should be fixed by this PR.

@raducotescu
Copy link
Contributor Author

raducotescu commented Dec 30, 2021

Neither the fix from #10766 nor this will address other browsers than Safari on iPhone/iPad, where the apps are forced to use the WebKit version provided by Apple. This is because the user agents are different for each browser:

  1. Chrome on iOS: Mozilla/5.0 (iPhone; CPU iPhone OS 15_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/96.0.4664.116 Mobile/15E148 Safari/604.1
  2. Firefox on iOS: Mozilla/5.0 (iPhone; CPU iPhone OS 15_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/40.2 Mobile/15E148 Safari/605.1.15

I guess we could also parse the iPhone OS 15_2 like Mac OS X part, but I don't have an iPad that could run the latest iPadOS so that I could verify the fix.

…iOS 15.2 / macOS 12.1 in the companion apps

* added iPadOS to the os types
…iOS 15.2 / macOS 12.1 in the companion apps

* simplified regex for matching the companion app
@raducotescu
Copy link
Contributor Author

raducotescu commented Jan 6, 2022

According to [1] and [2] the user agent used by both Firefox and Chrome on an iPad will be equivalent to the Safari desktop user agent. However, it seems that the Version is not reported correctly, so any kind of detection based on the user agent for these two browsers will fail.

In the end, I think we will have to accept this PR in its current state, which at least makes Safari on iOS >= 15.2 and macOS >= 12.1 plus the companion app to render the gauge cards correctly.

[1] - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent/Firefox
[2] - https://developer.chrome.com/docs/multidevice/user-agent/#chrome-for-ios

@zsarnett
Copy link
Contributor

zsarnett commented Jan 6, 2022

Ill take a look at this later today or tomorrow. Thanks!

@bramkragten
Copy link
Member

I suggest we remove all animation from the gauge in Safari. This is getting way too messy.

@balloob
Copy link
Member

balloob commented Jan 11, 2022

If this workaround is only for animations, then I agree.

@raducotescu
Copy link
Contributor Author

raducotescu commented Jan 11, 2022

I suggest we remove all animation from the gauge in Safari. This is getting way too messy.

How would you then render the gauge card on Safari?

If this workaround is only for animations, then I agree.

The "workaround" is not for the animation itself, but rather for the way the gauge "fill" area is rendered.

@raducotescu
Copy link
Contributor Author

Can we have a decision here, please? Whatever the decision will be, it’s better than not doing anything. I really want to use gauge cards for my climate dashboard and all of my devices are Apple ones.

If you decide to merge the PR that’s great, if not I can obviously fork the gauge card and create a custom one for my instance, but this limbo state doesn’t help anyone.

@bramkragten
Copy link
Member

From what I can remember, the issues with Safari where with the way we animate the gauge, so you are saying the issues are still there if you remove all animations?

@raducotescu
Copy link
Contributor Author

@bramkragten, the issue is not caused by ha-card > ha-gauge > svg > animatetransform, but rather by how the transform is applied to ha-card > ha-gauge > svg > path.

Now that I pay more attention to it, there isn't any initial animation in Safari 15.2 and above, the way you defined in #6492.

@bramkragten
Copy link
Member

This is the solution probably to not have to check for Safari anymore (this is just for the none needle gauge):

diff --git a/src/components/ha-gauge.ts b/src/components/ha-gauge.ts
index 9ce125966..64852ac68 100644
--- a/src/components/ha-gauge.ts
+++ b/src/components/ha-gauge.ts
@@ -65,12 +65,12 @@ export class Gauge extends LitElement {
 
   protected render() {
     return svg`
-      <svg viewBox="0 0 100 50" class="gauge">
+      <svg viewBox="-50 -50 100 50" class="gauge">
         ${
           !this.needle || !this.levels
             ? svg`<path
           class="dial"
-          d="M 10 50 A 40 40 0 0 1 90 50"
+          d="M -40 0 A 40 40 0 0 1 40 0"
         ></path>`
             : ""
         }
@@ -87,9 +87,9 @@ export class Gauge extends LitElement {
                         stroke="var(--info-color)"
                         class="level"
                         d="M
-                          ${50 - 40 * Math.cos((angle * Math.PI) / 180)}
-                          ${50 - 40 * Math.sin((angle * Math.PI) / 180)}
-                         A 40 40 0 0 1 90 50
+                          ${0 - 40 * Math.cos((angle * Math.PI) / 180)}
+                          ${-50 - 40 * Math.sin((angle * Math.PI) / 180)}
+                         A 40 40 0 0 1 40 25
                         "
                       ></path>`;
                   }
@@ -125,17 +125,8 @@ export class Gauge extends LitElement {
               `
             : svg`<path
                 class="value"
-                d="M 90 50.001 A 40 40 0 0 1 10 50"
-                style=${ifDefined(
-                  !isSafari
-                    ? styleMap({ transform: `rotate(${this._angle}deg)` })
-                    : undefined
-                )}
-                transform=${ifDefined(
-                  isSafari
-                    ? `rotate(${this._angle}${isSafari152 ? "" : " 50 50"})`
-                    : undefined
-                )}
+                d="M -40 0 A 40 40 0 1 0 40 0"
+                style=${styleMap({ transform: `rotate(${this._angle}deg)` })}
               >`
         }
         ${
@@ -187,7 +178,6 @@ export class Gauge extends LitElement {
         fill: none;
         stroke-width: 15;
         stroke: var(--gauge-color);
-        transform-origin: 50% 100%;
         transition: all 1s ease 0s;
       }
       .needle {

@raducotescu
Copy link
Contributor Author

Let me test it on my setup and see if works correctly in all the browsers/apps that can run on my Mac. It would be cool if we could get rid of the Safari-specific code, but we should also make sure that people who haven't updated to the latest Safari don't experience any issues.

@bramkragten
Copy link
Member

It should work on all browsers, it shifts everything so the rotation origin is in the 0,0 of the viewport. That was the issue in Safari in the first place.

@raducotescu
Copy link
Contributor Author

It seems to render correctly on all Safari versions I have access to: 15.1, 15.2, 15.3. The needle code will have to be adapted though and that's where I'm stuck. :D

@bramkragten
Copy link
Member

You should change all x and y coordinates to 50 less

@raducotescu
Copy link
Contributor Author

Thanks for the hint, @bramkragten! I have a slightly different patch based on what you started and it seems to work correctly on Safari >= 15.1 (it doesn't affect the Safari 15.1 rendering and also works correctly on Safari 15.2 and above). I want to test it on iOS 14 as well and, if successful, will open a new PR.

@raducotescu
Copy link
Contributor Author

Let's close this PR in favour of #11363, which is way cleaner.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 20, 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.

The gauge card doesn't render correctly on iOS 15.2 / macOS 12.1 in the companion apps
5 participants