The Screen Orientation API #7

Closed
marcoscaceres opened this Issue Jul 22, 2013 · 4 comments

Projects

None yet

2 participants

@marcoscaceres
Member

Quick review:

  • the somewhat liberal use of SHOULD in that spec is going to lead to user agents doing bad things. It basically says that UAs are allowed to return bogus orientation values. This will lead to Device Orientation all over again (where UA's returned laughably different values depending on which way one rotates their phone).
  • overloading the lockOrientation() method could just use the "or" operator in WebIDL.
  • The unlockOrientation() and lockOrientation() methods do the same thing. The methods should be merged into a setOrientation() method. Setting the orientation to null or the empty string just returns it to its default. Could also add a "default" orientation keyword.
  • If the spec can be changed to setOrientation([TreatEmptyStringAs=null] Orientation value), then it should vend a Promise. So then, for example:
setOrientation("portrait").then(showSplashScreen, fail);
setOrientation("landscape").then(showGameMenu, fail); 
//unlock it
setOrientation(null).then(whatever); 
  • The allowed orientation values need to be defined as an enum.
  • The spec does not define which task queue to use.
  • The spec treats Screen as extending EventTarget, but Screen is not an EventTarget. Either the spec needs to make Screen and EventTarget or CSSOM View needs to be updated to be an EventTarget.

Comments?

@marcoscaceres
Member

I've moved the above into a document:
https://github.com/w3ctag/spec-reviews/blob/master/2013/07/OrientationLock.md

PR's welcome!

@mounirlamouri

I think we could close this bug:

  • I only kept a few SHOULD around the orientation names because I think some UAs might want to not implement some of them or even add them. We could replace that by MUST but given that a UA can simply ignore a specific orientation all the time, it seems useless.
  • I'm using "or" for lockOrientation.
  • I do not agree with setOrientation(): it is confusing and does not given much. Android does something like that and you would be surprise to see how many developers ask "how do I unlock?".
  • I opened a bug for the enum: https://www.w3.org/Bugs/Public/show_bug.cgi?id=24695
  • Task source is now defined.
  • EventTarget has been added.
@marcoscaceres
Member

@mounirlamouri and I went through everything. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment