macosui / macos_ui

Flutter widgets and themes implementing the current macOS design language.
https://macosui.github.io/macos_ui/#/
MIT License
1.87k stars 182 forks source link

make CapacityIndicator work with other values of splits, not only splits:10 #305

Closed schilken closed 1 year ago

schilken commented 2 years ago

Currently the CapacityIndicator only works with the default value of 10 for splits.

I changed CapacityIndicator to work with other values and IndicatorsPage now uses splits:20.

macos_ui git:(dev) sh ./pr_prelaunch_tasks.sh
Formatted 115 files (0 changed) in 0.45 seconds.
Run dart fix --dry-run? [y/n]
y
Computing fixes in macos_ui (dry run)... 10.5s
Nothing to fix!
Run dart fix --apply? [y/n]
y
Computing fixes in macos_ui... 10.4s
Nothing to fix!
No changes to commit
Run tests? [y/n]
y
00:05 +15: /Users/aschilken/flutterdev/forks/macos_ui/test/buttons/popup_button_test.dart: MacosPopupButton tests debugFillProperties
[itemHeight: 24.0, noAutofocus, popupColor: null, menuMaxHeight: null, alignment: AlignmentDirectional.centerStart]
00:18 +119: All tests passed!
GroovinChip commented 2 years ago

@whiplashoo is Capacity Indicator something still used by macOS devs? I can't find a reference to it in the HIG.

GroovinChip commented 1 year ago

@whiplashoo Pinging you again for your thoughts on this

schilken commented 1 year ago

@GroovinChip Hi, sorry for not responding. Surely I use it, and I found this:
https://developer.apple.com/design/human-interface-guidelines/components/status/level-indicators/

schilken commented 1 year ago

@GroovinChip I added an entry in CHANGELOG and a unit test for splits= 20. I don't know what you mean by "update the version".

I think the unit tests of the CapacityIndicator don't prove anything useful concerning the splits, they just check the properties, not the calculation of the segments, but I have no idea who to test that.

On the IndicatorsPage of the example I changed the splits to 20 to show that the bug is fixed.

GroovinChip commented 1 year ago

I'll take a look at the tests when I get a moment.

By "update the version" I'm talking about incrementing the version number in the package's pubspec.yaml.

schilken commented 1 year ago

Hi @GroovinChip, I added two tests to check the number of segments. Thanks for pointing out the flutter tests. That was very helpful; I did not know the canvas_mock yet.

GroovinChip commented 1 year ago

@schilken looks like you need to format capacity_indicators_test.dart, mock_canvas.dart, and recording_canvas.dart

GroovinChip commented 1 year ago

@schilken Can you pull in the latest from dev so I can re-review, please?

schilken commented 1 year ago

After merging the upstream dev branch into my local branch, I get an exception when clicking the CapacityIndicator on the IndicatorsPage of the example app:

Exception has occurred.
_AssertionError ('package:macos_ui/src/indicators/slider.dart': Failed assertion: line 40 pos 16: 'value >= min && value <= max': is not true.)

The ranges 0-100 of CapacityIndicator and 0.0 to 1.0 of Slider are not compatible.

A further test reveals, that the release build just works fine, it's just the connection of CapacityIndicator and slider that triggers the assertError.

GroovinChip commented 1 year ago

After merging the upstream dev branch into my local branch, I get an exception when clicking the CapacityIndicator on the IndicatorsPage of the example app:


Exception has occurred.

_AssertionError ('package:macos_ui/src/indicators/slider.dart': Failed assertion: line 40 pos 16: 'value >= min && value <= max': is not true.)

The ranges 0-100 of CapacityIndicator and 0.0 to 1.0 of Slider are not compatible.

A further test reveals, that the release build just works fine, it's just the connection of CapacityIndicator and slider that triggers the assertError.

It's just a problem in the example app, I address it in #342.