Add permissionStatus property#32
Add permissionStatus property#32FluorescentHallucinogen wants to merge 3 commits intoebidel:masterfrom
permissionStatus property#32Conversation
| "location", | ||
| "position", | ||
| "navigation", | ||
| "api", |
There was a problem hiding this comment.
don't think we need these. they're vague
There was a problem hiding this comment.
@ebidel I've removed the permission keyword. But IMO we should keep the api keyword. <geo-location> web component is a part of my progressive-elements collection that is a set of web components for modern web APIs such as Payment Request API, Media Session API, etc. People can find these web components by the api keyword.
| navigator.permissions.query({ | ||
| name: 'geolocation' | ||
| }).then(function(permissionStatus) { | ||
| this._setPermissionStatus(permissionStatus.state); |
There was a problem hiding this comment.
need to handle the permissions api not being available.
There was a problem hiding this comment.
@ebidel What should we do if Permissions API is not available?
There was a problem hiding this comment.
Probably fail silently, but the permissionStatus needs a default value or some way to signal to users that permission state is unknown.
| permissionStatus: { | ||
| type: String, | ||
| readOnly: true, | ||
| notify: true |
There was a problem hiding this comment.
@ebidel Is not the default value optional? Should I use value: null?
There was a problem hiding this comment.
null sounds good. As long as that's not one of the possible values the API returns.
There was a problem hiding this comment.
I mean that if we do not specify a default value at all, it is equivalent to null. Should I specify it explicitly?
| name: 'geolocation' | ||
| }).then(function(permissionStatus) { | ||
| this._setPermissionStatus(permissionStatus.state); | ||
| permissionStatus.onchange = function() { |
There was a problem hiding this comment.
What happens if this element gets attached/detached/attached from the DOM? Does permissionStatus get gc'd properly and we're not adding creating a bunch of orphaned change handlers?
There was a problem hiding this comment.
@ebidel I don't know. Could you please help me do this in a better way?
There was a problem hiding this comment.
As the author of this PR, I was hoping you could do that research.
There was a problem hiding this comment.
@ebidel BTW, what about placing this code inside ready lifecycle callback instead of attached? Is it a bad idea?
attached can be called multiple times during the lifetime of an element. The first attached callback is guaranteed not to fire until after ready.
2 participants
permissionStatusproperty that indicates whether Geolocation API permission status isgranted,deniedorprompt.geo-location.html,demo/index.htmlandREADME.mdfiles.apiandpermissionkeywords tobower.jsonfile.GelocationtoGeolocationeverywhere).