material-foundation / material-color-utilities

Color libraries for Material You
Apache License 2.0
1.57k stars 134 forks source link

Dart version: `TonalPalette.getHct` throws "Null check operator used on a null value" when created with `TonalPalette.fromList` #107

Open rydmike opened 12 months ago

rydmike commented 12 months ago

TonalPalette.getHct throws "Null check operator used on a null value"

How to reproduce issue

To repeat the issue, in "palettes_test.dart" test group('[.fromList constructor]' insert the following example tests:

    group('[.fromList constructor]', () {
      test('tones of i', () async {
        final List<int> ints =
            List<int>.generate(TonalPalette.commonSize, (int i) => i);
        final TonalPalette tones = TonalPalette.fromList(ints);

        expect(tones.get(100), 12);
        expect(tones.get(99), 11);
        expect(tones.get(95), 10);
        expect(tones.get(90), 9);
        expect(tones.get(80), 8);
        expect(tones.get(70), 7);
        expect(tones.get(60), 6);
        expect(tones.get(50), 5);
        expect(tones.get(40), 4);
        expect(tones.get(30), 3);
        expect(tones.get(20), 2);
        expect(tones.get(10), 1);
        expect(tones.get(0), 0);

        /// Tone not in [TonalPalette.commonTones]
        expect(() => tones.get(3), throwsA(isA<ArgumentError>()));

        // NEW TESTS
        // Test getHct with HCT TonalPalette tones.
        // These throw "Null check operator used on a null value"
        expect(tones.getHct(0).toInt(), 0);
        expect(tones.getHct(10).toInt(), 1);
        expect(tones.getHct(20).toInt(), 2);
        expect(tones.getHct(30).toInt(), 3);
        expect(tones.getHct(40).toInt(), 4);
        expect(tones.getHct(50).toInt(), 5);
        expect(tones.getHct(60).toInt(), 6);
        expect(tones.getHct(70).toInt(), 7);
        expect(tones.getHct(80).toInt(), 8);
        expect(tones.getHct(90).toInt(), 9);
        expect(tones.getHct(95).toInt(), 10);
        expect(tones.getHct(99).toInt(), 11);
        expect(tones.getHct(100).toInt(), 12);

        // This is OK, throws as it should
        expect(() => tones.getHct(3), throwsA(isA<ArgumentError>()));
      });

Issue

There is no branch handling the getHct case for when TonalPalette was created with fromList.

  /// Get the HCT color for a given tone.
  Hct getHct(double tone) {
    if (_hue == null || _chroma == null) {
      if (!_cache.containsKey(tone)) {
        throw ArgumentError.value(
          tone,
          'tone',
          'When a TonalPalette is created with fromList, tone must be one of '
              '$commonTones',
        );
      } 
    }
    return Hct.from(_hue!, _chroma!, tone);
  }

Fix

To fix the issue the return this case must be added.

  /// Get the HCT color for a given tone.
  Hct getHct(double tone) {
    if (_hue == null || _chroma == null) {
      if (!_cache.containsKey(tone)) {
        throw ArgumentError.value(
          tone,
          'tone',
          'When a TonalPalette is created with fromList, tone must be one of '
              '$commonTones',
        );
      // ADD: Return HCT from the int tone in the cache. 
      } else {
        return Hct.fromInt(_cache[tone]!);
      }
    }
    return Hct.from(_hue!, _chroma!, tone);
  }