Improve HTMLCollection, add HTMLOptionsCollection and HTMLFormControlsCollection

[ghostbuster91]

Hi,

Following suggestion from @raquo here is a PR which fixes incorrect type of options property in HTMLSelectElement.

Cheers!

[armanbilge]

Thanks for contributing!! At a first glance this is looking good.

Looking at MDN:
https://developer.mozilla.org/en-US/docs/Web/API/HTMLOptionsCollection
https://developer.mozilla.org/en-US/docs/Web/API/HTMLCollection

I wonder if we should define HTMLCollection with the item and namedItem methods, and then HTMLOptionsCollection can extend that. What do you think?

[ghostbuster91]

That was my first thought but than I noticed private constructor and got confused by all of these js annotations. I don't really know much about scala-js.

Let me know if I did it right :)

[armanbilge]

Moving the conversation from #593 (comment) back to the main thread...

It seems that typescript does use Element as an upper bound for HTMLCollection.

/** A generic collection (array-like object similar to arguments) of elements (in document order) and offers methods and properties for selecting from the list. */
interface HTMLCollectionBase {
    /**
     * Sets or retrieves the number of objects in a collection.
     */
    readonly length: number;
    /**
     * Retrieves an object from various collections.
     */
    item(index: number): Element | null;
    [index: number]: Element;
}

interface HTMLCollection extends HTMLCollectionBase {
    /**
     * Retrieves a select object or an object from an options collection.
     */
    namedItem(name: string): Element | null;
}

declare var HTMLCollection: {
    prototype: HTMLCollection;
    new(): HTMLCollection;
};

[armanbilge]

Then they have:

interface HTMLFormControlsCollection extends HTMLCollectionBase {
    /**
     * Returns the item with ID or name name from the collection.
     *
     * If there are multiple matching items, then a RadioNodeList object containing all those elements is returned.
     */
    namedItem(name: string): RadioNodeList | Element | null;
}

I think they are doing really tricky typescripty things we can't replicate in Scala actually 🤔

Given the MDN docs, I think it's okay to remove the upper bound.

[armanbilge]

Ah, the "trick" is that they are extending HTMLCollectionBase (with the Element upper-bound) instead of HTMLCollection, and then manually defining namedItem with non-Element return type (rather than overriding). Which actually we could do too I guess.

I'd still rather remove the upper bound. It's cleaner and supported by the docs.

[armanbilge]

Just a couple tiny things...

Otherwise, looks great!! Thank you so much for taking this on and sticking with it, much appreciated. This is a significant improvement to this API, and just in time for 2.0.0 😁

[ghostbuster91]

Otherwise, looks great!! Thank you so much for taking this on and sticking with it, much appreciated. This is a significant improvement to this API, and just in time for 2.0.0 grin

No problem at all. I learned some new things and I am always happy to contribute back to scala ecosystem :)
Thanks for the guidance 👍

[armanbilge]

Thanks again!

[armanbilge]

@japgolly do you want to have a look at this?

[japgolly]

Fantastic changes! Just one question about variance...

[japgolly]

Should this be HTMLCollection[+E <: Node] extends DOMList[E]?

Note: If yes, don't forget to add a + to the type alias too :)

[armanbilge]

I'm interpreting the spec as any object? Not just a Node?
https://developer.mozilla.org/en-US/docs/Web/API/HTMLCollection

The HTMLCollection interface represents a generic collection (array-like object similar to arguments) of elements (in document order) and offers methods and properties for selecting from the list.

[armanbilge]

The + makes sense though 👍