Skip to content

Conversation

yoavweiss
Copy link
Contributor

This is the HTML side of whatwg/dom#103

@annevk/@zcorpan - can you take a look?

source Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I would make this relList's DOMTokenList. Also, rather than "include" I would use "are" and rather than "rel values" I think the word we use is "keywords". Also, after "HTML link types" I would add "that are supported by the implementation".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, will change accordingly.

@zcorpan
Copy link
Member

zcorpan commented Nov 18, 2015

LGTM but will let @annevk take another look. (The "other specifications" thing looks unresolved.)

@yoavweiss
Copy link
Contributor Author

@annevk let me know if you approve or the "other specifications" part needs to be removed. Afterwards, I'll rebase and squash.

source Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think you should include DOMTokenList here. As in "relList's DOMTokenList/DOMSettableTokenList?'s supported tokens". They are not associated with relList directly.

@yoavweiss
Copy link
Contributor Author

@annevk I believe I addressed everything you raised, but please verify :)

source Outdated
Copy link
Member

Choose a reason for hiding this comment

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

You should pick the one relList actually returns here.

@annevk
Copy link
Member

annevk commented Nov 19, 2015

Apart from that space, it seems the wrapping is not really correct. If you want I can address both. (Or whoever merges this.)

Add the following:
* Reference to DOMTokenList's supported tokens in the DOM spec.
* Supported tokens to HTMLLinkElement's relList.
* Supported tokens to HTMLIFrameElement's sandbox.
@yoavweiss
Copy link
Contributor Author

Fixed wrapping and space as well as squashed

@zcorpan zcorpan closed this in f7a66ed Nov 19, 2015
@zcorpan
Copy link
Member

zcorpan commented Nov 19, 2015

Thank you! I tweaked it some more. :-)

zcorpan added a commit that referenced this pull request Nov 19, 2015
Rename dom-tokenlist-supported-tokens to concept-supported-tokens
since that's the internal term DOM uses. Also rewrap the text.

I had missed to `add` this change when merging
f7a66ed.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants