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

style: 'percent' vs {style: 'unit' unit: 'percent'} #44

Closed
FrankYFTang opened this issue May 16, 2019 · 6 comments · Fixed by #57
Closed

style: 'percent' vs {style: 'unit' unit: 'percent'} #44

FrankYFTang opened this issue May 16, 2019 · 6 comments · Fixed by #57

Comments

@FrankYFTang
Copy link
Contributor

What will be the differences between

  1. {style: 'percent'} AND
  2. {style: 'unit', unit: 'percent'}

? in particular, what should the resolvedOptions report about the style and unit for these 2 cases. I found it is hard to distinguish between them.

@sffc
Copy link
Collaborator

sffc commented May 16, 2019

{style: "percent"} has a couple legacy code paths:

If style is "percent", then
  Let mxfdDefault be 0.
Else,
  Let mxfdDefault be 3.

…

If numberFormat.[[Style]] is "percent", let x be 100 × x.

Otherwise, you can treat them the same.

In terms of resolvedOptions, the way the spec is written, resolvedOptions().style === "percent" implies that resolvedOptions().unit === "percent". That might not make sense actually. It would not be hard to make the spec not set .unit when style === "percent". I should make a PR for that.

@sffc
Copy link
Collaborator

sffc commented May 16, 2019

Actually, one open question would be whether we want to enable unitWidth when style === "percent". I can be convinced either way on that detail. Maybe we just want to keep style === "percent" as a legacy thing, and keep unitWidth for the new unit syntax.

These changes require a PR.

@FrankYFTang
Copy link
Contributor Author

ah... the scale 100 will tell me the differences. That is good enough for me in the spec level.

@sffc
Copy link
Collaborator

sffc commented May 16, 2019

I want to reopen this issue to address the point about unitWidth. I think it would be more clean to leave style: "percent" as a standalone legacy, and not get it confused with the new unitWidth construction. What do you think?

@sffc sffc reopened this May 16, 2019
@littledan
Copy link
Member

I like this idea in #44 (comment) . I hadn't noticed the redundancy. Is there a rationale for the current state?

@sffc
Copy link
Collaborator

sffc commented May 17, 2019

Is there a rationale for the current state?

My first draft of the spec text had the "UnitType" construction for percents, which based on feedback I removed in favor of keeping just "Style". I think this is a remnant from that change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants