Skip to content

Conversation

darrnshn
Copy link
Collaborator

issue #482
Hi @tabatkins, PTAL. I'm not sure if I'm doing this right, so I only wrote up the interface for now. I'll do the actual text once we've agreed on the interface.

@darrnshn
Copy link
Collaborator Author

darrnshn commented Jan 22, 2018

@tabatkins Can you PTAL at the proposed IDL when you have time? Thanks!

@darrnshn
Copy link
Collaborator Author

Note that whether the base types should be required or optional depends on whether we need to differentiate between the types { length: 0 } and {}. I think in the original IRC log, you wanted to differentiate between e.g. 1px/1px and 1? How important is this? IMO it's much easier to implement arithmetic if we don't have to differentiate between a "null" exponent and a zero exponent.

I don't think this has any observable impact on arithmetic, so only CSSNumericValue.type() can tell the difference between a null and a zero exponent.

Maybe @shans can weigh in on this as well since I asked him a similar question a while back.

@tabatkins
Copy link
Member

Nah, I can't find why I would have wanted to distinguish a zero exponent from a null exponent. All the algorithms are careful to only talk about non-zero exponents.

And I'd probably want .type() to only emit the non-zero exponents, too.

@darrnshn
Copy link
Collaborator Author

Thanks, I think @shans mentioned that animations distinguish between zero/null or something, but I don't know how useful that is for Typed OM.

Anyway, it looks like exponents should be optional either way. We can always distinguish zero/null later with no impact on the API.

@tabatkins
Copy link
Member

Yeah, we want to preserve zero values in a calc(), so the "shape" remains stable when animating across 0, but that reasoning doesn't apply to zero exponents in the type; those don't change in animations. ^_^

Copy link
Collaborator Author

@darrnshn darrnshn left a comment

Choose a reason for hiding this comment

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

Added some questions/comments inline.


<div algorithm="CSSNumericValue.type()">
The <dfn method for=CSSNumericValue>type()</dfn> method
returns a representation of the [=type=] of |this|.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might be better ways of saying this.

Copy link
Member

Choose a reason for hiding this comment

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

nah, seems fine


When called, it must perform the following steps:

1. Let |result| be a new {{CSSNumericType}} with all attributes set to `null`.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can I omit the "with all attributes set to null" since it's probably the default behaviour?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah.


2. For each |baseType| → |power| in the [=type=] of |this|,
1. If |power| is not 0,
set |result|'s |baseType| attribute to |power|.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if attribute is the correct term. Maybe slot?

Copy link
Member

Choose a reason for hiding this comment

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

The term for dictionaries is "member". Alternately, since per WebIDL you're allowed to treat dictionaries as ordered maps, you can just say "set |result|[|baseType|] to |power|".

set |result|'s |baseType| attribute to |power|.

3. If the [=percent hint=] of |this| is not null,
1. Set {{CSSNumericType/percentHint}} to the [=percent hint=] of |this|.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bit handwavy as it doesn't quite type check - CSSNumericType.percentHint is an enum whereas the percent hint of this is a string (I think).

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 that's fine - enums turn into strings when they go to ECMAScript anyway.

@darrnshn
Copy link
Collaborator Author

Cool, made the requested changes. PTAL

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants