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

On Safari 10 constructor check is broken #237

Merged
merged 4 commits into from Mar 15, 2017

Conversation

andrey-git
Copy link
Contributor

On Safari 10 don't compare constructors when checking if custom UI element is loaded.
That check doesn't work (always says "loaded")

@mention-bot
Copy link

@andrey-git, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @turbokongen and @persandstrom to be potential reviewers.

@@ -42,7 +42,11 @@
},

_maybeLoadCustomUi: function (stateType) {
var isLoaded = this._isLoaded('STATE-CARD-' + stateType.toUpperCase());
// Constructor check doesn't work on Safari 10.
// Bypassing this check mean that in order to user a built-in
Copy link
Member

Choose a reason for hiding this comment

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

Wait, this means that it is broken by default for Safari 10 users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, custom UI didn't work on Safari 10.
Yesterday someone lend me their browserstack account so I could debug it.

We could remove the isLoaded check altogether. The reason I put it in in the first place was to allow using built-in components as custom UI. I guess it is a very edge-case, and is still possible by placing an empty "custom" html to prevent loading error.

Copy link
Member

Choose a reason for hiding this comment

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

We should not allow people to use built-in components as custom ui. We should remove that functionality completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@balloob balloob merged commit 215217f into home-assistant:master Mar 15, 2017
tkdrob pushed a commit to tkdrob/frontend that referenced this pull request Apr 20, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 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.

None yet

4 participants