Skip to content

Add JS type defs for calculations#3605

Merged
nex3 merged 14 commits intosass:mainfrom
oddbird:js-api-calculations
Jun 20, 2023
Merged

Add JS type defs for calculations#3605
nex3 merged 14 commits intosass:mainfrom
oddbird:js-api-calculations

Conversation

@jerivas
Copy link
Contributor

@jerivas jerivas commented Jun 7, 2023

jerivas and others added 5 commits June 7, 2023 00:36
* main:
  [Ordered Comments] Fix the issue reference (sass#3606)
  [Ordered Comments] Mark as accepted (sass#3604)
  Update the embedded protocol to 2.0.0 (sass#3599)
@jerivas jerivas marked this pull request as ready for review June 15, 2023 15:33
@jerivas
Copy link
Contributor Author

jerivas commented Jun 15, 2023

@nex3 I think I addressed all your comments. This is ready for another round of review

@jerivas jerivas requested a review from nex3 June 15, 2023 15:35
Comment on lines +96 to +97
* Creates a Sass CalculationOperation by setting the fields to the arguments
* of the corresponding names, and returns it.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this still reads a bit spec-y. Maybe just "Creates a Sass CalculationOperation with the given [operator], [left], and [right] values."?

Comment on lines +125 to +126
* Creates a Sass CalculationInterpolation by setting the value field to the
* value argument and returns it.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar comment to the constructor above.

}

/**
* A string injected into a {@link SassCalculation} using interpolation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth mentioning here that interpolations are different from unquoted strings in that they're always surrounded in parentheses when they appear in CalculationOperations.

@jerivas jerivas requested a review from nex3 June 16, 2023 22:19
@nex3 nex3 merged commit 3991504 into sass:main Jun 20, 2023
nex3 added a commit that referenced this pull request Jun 20, 2023
This reverts commit 3991504. This
should only have been landed in parallel with the implementation and
tests.
nex3 added a commit that referenced this pull request Jun 21, 2023
This reverts the parts of commit 3991504 that modify the official specs. These should only have landed along with implementation updates.
@jerivas jerivas deleted the js-api-calculations branch September 11, 2023 22:27
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.

3 participants