Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Color 4] Dart JS API implementation #2117

Merged
merged 51 commits into from
Nov 17, 2023
Merged

Conversation

jamesnw and others added 22 commits September 25, 2023 22:56
* feature.color-4: (30 commits)
  Update for `lch()` tests (sass#2108)
  Add support for relative color syntax (sass#2112)
  Update for `oklab()` tests (sass#2094)
  Poke CI
  Cut a release (sass#2107)
  Implement first class mixins (sass#2073)
  Fix a race condition preventing embedded compiler to shutdown after a protocol error (sass#2106)
  Switch to the GitHub-hosted MacOS ARM64 runner (sass#2103)
  Update the version of Sass used by the website on release (sass#2102)
  Bump actions/checkout from 3 to 4 (sass#2088)
  Bump docker/setup-qemu-action from 2 to 3 (sass#2089)
  Implement support for the relative color syntax of CSS Color 5 (sass#2098)
  Rephrase errors for numbers that must be unitless or % (sass#2101)
  Forbid LLM contributions (sass#2100)
  Update protocol-version during embedded-host-node release (sass#2097)
  Deprecate Deprecation.calcInterp (sass#2096)
  Avoid useless allocations for interpolations without maps (sass#2095)
  Fix an error during embedded compiler shutdown (sass#2092)
  Cut a release (sass#2090)
  Expose the containing URL to importers under some circumstances (sass#2083)
  ...
@jamesnw
Copy link
Contributor Author

jamesnw commented Oct 13, 2023

@nex3 This is ready for an initial review. Thanks!

@jgerigmeyer jgerigmeyer requested a review from nex3 October 16, 2023 16:01
CHANGELOG.md Outdated Show resolved Hide resolved
lib/src/evaluation_context.dart Outdated Show resolved Hide resolved
lib/src/evaluation_context.dart Outdated Show resolved Hide resolved
lib/src/js/value/color.dart Outdated Show resolved Hide resolved
lib/src/js/value/color.dart Outdated Show resolved Hide resolved
lib/src/js/value/color.dart Outdated Show resolved Hide resolved
lib/src/js/value/color.dart Outdated Show resolved Hide resolved
lib/src/js/value/color.dart Outdated Show resolved Hide resolved
@jamesnw jamesnw requested a review from nex3 October 26, 2023 19:18
lib/src/js/value/color.dart Outdated Show resolved Hide resolved
Co-authored-by: Natalie Weizenbaum <[email protected]>
@jgerigmeyer jgerigmeyer marked this pull request as ready for review November 2, 2023 18:13
@jgerigmeyer jgerigmeyer requested a review from nex3 November 2, 2023 20:01
@nex3
Copy link
Contributor

nex3 commented Nov 3, 2023

It looks like the powerless channel tests need to be adjusted for sass/sass#3654

@jgerigmeyer
Copy link
Contributor

It looks like the powerless channel tests need to be adjusted for sass/sass#3654

@nex3 Updated in sass/sass-spec@bc2bd0b

I may be missing something, but it looks to me like the the remaining failing tests are cases where Dart Sass is trailing the spec (e.g. powerless channels, clamped channels, legacy color equality). Let me know if you think that's incorrect!

@nex3
Copy link
Contributor

nex3 commented Nov 6, 2023

That's very plausible. Feel free to just mark those tests as todo for now and I'll reenable them as I get the implementation into shape.

_toSpace(self, space).toGamut(),
'toGamut': (SassColor self, [String? space]) {
var originalSpace = self.space;
return _toSpace(self, space).toGamut().toSpace(originalSpace);
Copy link
Contributor

@jgerigmeyer jgerigmeyer Nov 6, 2023

Choose a reason for hiding this comment

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

That's very plausible. Feel free to just mark those tests as todo for now and I'll reenable them as I get the implementation into shape.

One of the tests was failing because of this bug, but I added comments to the rest of the failing tests (and skipped them for dart-sass for now), which I think are all caused by implementation lagging spec changes.

* feature.color-4:
  Poke CI
  Update link to Node.js releases page (sass#2131)
  Write implementation documentation (sass#2042)
  Remove dead code (sass#2129)
  Add compatibility with Node.js 21.0.0 (sass#2128)
  Bump lints from 2.1.1 to 3.0.0 (sass#2126)
  Bump dartdoc from 6.3.0 to 7.0.0 (sass#2118)
  Cut a release (sass#2120)
  Update pubspec and changelog for sass/embedded-host-node#257 (sass#2116)
  Fix crash in browser when running alongside NextJS (sass#2114)
  Cut another release (sass#2111)
@nex3 nex3 merged commit c91178d into sass:feature.color-4 Nov 17, 2023
40 checks passed
@jgerigmeyer jgerigmeyer deleted the color-spaces-js branch November 18, 2023 02:55
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.

None yet

3 participants