Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

isLocked: attribute or function? #43

Closed
vmpstr opened this issue Feb 11, 2019 · 1 comment
Closed

isLocked: attribute or function? #43

vmpstr opened this issue Feb 11, 2019 · 1 comment

Comments

@vmpstr
Copy link
Collaborator

vmpstr commented Feb 11, 2019

From https://chromium-review.googlesource.com/c/chromium/src/+/1464202

We've added an isLocked attribute, but perhaps it should be a function to make it clear that it's not a settable attribute. Not sure what the best js style is here.

@domenic for an opinion.
@domenic
Copy link

domenic commented Feb 12, 2019

Generally we use getters (i.e. readonly attributes) for this sort of situation. Also, we omit the "is" (so just locked), but that's more loose. (See https://w3ctag.github.io/design-principles/#api-surface last paragraph.)

aarongable pushed a commit to chromium/chromium that referenced this issue Feb 15, 2019
@vmpstr
[DL]: Rename isLocked to locked as per design principle.
0582192 
As https://w3ctag.github.io/design-principles/#api-surface suggests,
boolean attributes should not have "is" at the beginning of their names.

This patch fixes this for DisplayLockContext.isLocked

R=chrishtr@chromium.org

Bug: 882663, github.com/WICG/display-locking/issues/43
Change-Id: Ie5be4b4f6fd0ee8656fbb3c31aae0e7eb0fc3612
Reviewed-on: https://chromium-review.googlesource.com/c/1466155
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632467}
@vmpstr vmpstr closed this as completed Mar 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@domenic @vmpstr