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

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

Merged
merged 18 commits into from
Feb 11, 2023

Conversation

schilken
Copy link
Contributor

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
Copy link
Collaborator

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

@GroovinChip
Copy link
Collaborator

@whiplashoo Pinging you again for your thoughts on this

@schilken
Copy link
Contributor Author

@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/

Copy link
Collaborator

@GroovinChip GroovinChip left a comment

Choose a reason for hiding this comment

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

Looks good. Please add a test that accounts for varying amounts of steps, and update the version & the change log.

@schilken
Copy link
Contributor Author

@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
Copy link
Collaborator

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.

Copy link
Collaborator

@GroovinChip GroovinChip left a comment

Choose a reason for hiding this comment

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

OK @schilken I was able to review the test you wrote. I think you should take a look at how Flutter tests the ProgressIndicator widget. Check it out here and add some similar tests. I don't think you need to test it as deeply as they do, but it should give you some ideas and direction.

@schilken
Copy link
Contributor Author

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 GroovinChip self-requested a review January 27, 2023 10:47
@GroovinChip
Copy link
Collaborator

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

@GroovinChip
Copy link
Collaborator

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

@schilken
Copy link
Contributor Author

schilken commented Feb 5, 2023

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
Copy link
Collaborator

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.

@GroovinChip GroovinChip dismissed their stale review February 9, 2023 22:00

Upstream has been merged in

Copy link
Collaborator

@GroovinChip GroovinChip left a comment

Choose a reason for hiding this comment

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

Just a minor nit in the changelog, & please run flutter pub get to ensure the lockfiles stay updated. We'll be good to go after that 👍

Edit: also please address the failing check

CHANGELOG.md Outdated Show resolved Hide resolved
schilken and others added 3 commits February 11, 2023 10:59
@GroovinChip GroovinChip self-requested a review February 11, 2023 20:55
@GroovinChip GroovinChip dismissed their stale review February 11, 2023 20:55

to address new changes

Copy link
Collaborator

@GroovinChip GroovinChip left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@GroovinChip GroovinChip merged commit 9f8e372 into macosui:dev Feb 11, 2023
GroovinChip added a commit that referenced this pull request Feb 13, 2023
* Sidebar top (#244)

* chore: refactor dir structure

* feat: Sidebar top & updated default control color

* feat(example): search results in top

* chore: bump version, changelog

* chore: run flutter pub upgrade

* Update CHANGELOG.md

* Update lib/src/layout/sidebar/sidebar.dart

Co-authored-by: Minas Giannekas <whiplashoo@users.noreply.github.com>

* chore: update issue templates

* chore: update pr_prelaunch script

* Flutter 3 upgrade & MacosColor update (#248)

* chore: Update pubspec.yaml files to support Flutter 3

* chore: run dart fix --apply

* chore: migrate Scrollbar to Flutter 3

* chore: update flutter_lints & subsequent fixes

* feat: add missing functions to MacosColor

the Color class has a number of functions that MacosColor had not implemented

* chore: use super parameters

* chore: update changelog

* chore: tweak example app

Uses the new PlatformMenuBar. Also update product name.

* chore: small changelog tweak

* chore: run flutter format .

* chore: run dart fix --apply

* chore: run flutter format .

* chore: remove unused code in example

* chore: remove unused import

* chore: remove unused code

* Starter app (#251)

* chore: Update pubspec.yaml files to support Flutter 3

* chore: run dart fix --apply

* chore: migrate Scrollbar to Flutter 3

* chore: update flutter_lints & subsequent fixes

* feat: add missing functions to MacosColor

the Color class has a number of functions that MacosColor had not implemented

* chore: use super parameters

* chore: update changelog

* chore: tweak example app

Uses the new PlatformMenuBar. Also update product name.

* chore: small changelog tweak

* chore: run flutter format .

* chore: run dart fix --apply

* chore: run flutter format .

* chore: remove unused code in example

* chore: remove unused import

* feat: first pass at starter app brick

* chore: improve starter app brick

* chore: fix widget test in starter app

* feat: conditional prompts & running pub get

* chore: finalize brick

* chore: run flutter format

* chore: exclude starter app from analyzer

* Full screen opaque toolbar issue (closes #249) (#252)

* fix: don't show app window toolbar when in full screen

* chore: update README

* chore: update brick app window code

* chore: update pubspec and changelog

* chore: update actions

Co-authored-by: Reuben Turner <groovinchip@gmail.com>

* Version 1.4.1 (#255)

* Sidebar top (#244)

* chore: refactor dir structure

* feat: Sidebar top & updated default control color

* feat(example): search results in top

* chore: bump version, changelog

* chore: run flutter pub upgrade

* Update CHANGELOG.md

* Update lib/src/layout/sidebar/sidebar.dart

Co-authored-by: Minas Giannekas <whiplashoo@users.noreply.github.com>

* chore: update issue templates

* chore: update pr_prelaunch script

* Flutter 3 upgrade & MacosColor update (#248)

* chore: Update pubspec.yaml files to support Flutter 3

* chore: run dart fix --apply

* chore: migrate Scrollbar to Flutter 3

* chore: update flutter_lints & subsequent fixes

* feat: add missing functions to MacosColor

the Color class has a number of functions that MacosColor had not implemented

* chore: use super parameters

* chore: update changelog

* chore: tweak example app

Uses the new PlatformMenuBar. Also update product name.

* chore: small changelog tweak

* chore: run flutter format .

* chore: run dart fix --apply

* chore: run flutter format .

* chore: remove unused code in example

* chore: remove unused import

* chore: remove unused code

* Starter app (#251)

* chore: Update pubspec.yaml files to support Flutter 3

* chore: run dart fix --apply

* chore: migrate Scrollbar to Flutter 3

* chore: update flutter_lints & subsequent fixes

* feat: add missing functions to MacosColor

the Color class has a number of functions that MacosColor had not implemented

* chore: use super parameters

* chore: update changelog

* chore: tweak example app

Uses the new PlatformMenuBar. Also update product name.

* chore: small changelog tweak

* chore: run flutter format .

* chore: run dart fix --apply

* chore: run flutter format .

* chore: remove unused code in example

* chore: remove unused import

* feat: first pass at starter app brick

* chore: improve starter app brick

* chore: fix widget test in starter app

* feat: conditional prompts & running pub get

* chore: finalize brick

* chore: run flutter format

* chore: exclude starter app from analyzer

* Full screen opaque toolbar issue (closes #249) (#252)

* fix: don't show app window toolbar when in full screen

* chore: update README

* chore: update brick app window code

* chore: update pubspec and changelog

* chore: update actions

Co-authored-by: Reuben Turner <groovinchip@gmail.com>

Co-authored-by: Minas Giannekas <whiplashoo@users.noreply.github.com>

* chore: repository, homepage fields

* chore: update readme

* feat(starter_app): Version 1.1.0

* feat(starter_app): multi-window support

* feat: starter_app 1.2.1

* chore: move brick to its own repo & go back to old pana action

* Expand remaining part of row in MacosListTile (#265)

* Expand remaining part of row #264

* Increment to version 1.4.2

* End sidebar (#267)

* chore: add missing trailing comma

* chore: improve MacosIconButton animation curve

* chore: remove false_secrets from pubspec.yaml

* feat: end sidebar

Also fixes the tests portion of the pr_prelaunch_tasks script

* feat: add "isEndSidebarShown" to MacosWindowScope

* fix: Correct the placement of the leading widget in disclosure sidebar items (#272)

* chore: Update changelog

* test: fix issues with date_picker_test

* Account for cases where the month and the day are the same
* Fix offstage warnings by removing tester taps that disabled the caret controls

* Update Actions (#279)

* Update flutter_analysis.yml

* Update dart_code_metrics.yaml

* Update gh_pages.yml

* Update pana_analysis.yml

* Update codecov.yaml

* fix syntax issue

* Version 1.6.0 - `MacosTabView` & `MacosSegmentedControl` (#273)

* feat: MacosTabView & MacosSegmentedControl

* chore: fixup scripts

* test: tests for segmented control & tab view

* chore: remove unused code

* chore: run flutter format .

* chore: bump code metrics

* docs: dartdoc updates

* docs: fix a documentation error

* chore: run flutter format .

* refactor: make active property of MacosTab optional, since it is handled via MacosSegmentedControl

* chore: fix import change

* refactor: change colors to match default native app design

* docs: update tab view screenshot in readme

* chore: update README example for MacosTabView

* chore: fix typo in MacosSegmentedControl

* chore: fix typo in MacosSegmentedControl docstring

* test: remove explicitly setting active property of MacosTabs

Co-authored-by: Minas Giannekas <whiplashoo721@gmail.com>

* Version 1.7.0: `MacosImageIcon` & sidebar updates (#274)

* feat: MacosTabView & MacosSegmentedControl

* chore: fixup scripts

* test: tests for segmented control & tab view

* chore: remove unused code

* chore: run flutter format .

* chore: bump code metrics

* docs: dartdoc updates

* docs: fix a documentation error

* chore: run flutter format .

* feat: MacosImageIcon & sidebar updates

* test: fix issues with date_picker_test

* Account for cases where the month and the day are the same
* Fix offstage warnings by removing tester taps that disabled the caret controls

* refactor: make active property of MacosTab optional, since it is handled via MacosSegmentedControl

* chore: fix import change

* refactor: change colors to match default native app design

* docs: update tab view screenshot in readme

* chore: fix README

* test: fix date picker test

Co-authored-by: Minas Giannekas <whiplashoo721@gmail.com>

* chore: fix typo in pr template

* feat: gh action to auto-generate releases on push to stable

* chore: update release action with latest from stable

* feat: add action to publish to pub

* docs: update readme

* Addresses #237
* Adds MacosColorWell to selectors section
* Adds code snippets for date & time pickers

* fix: 1.7.1

Fixes issue where end sidebar window breakpoint wasn’t being respected

* Tab view padding (#285)

* fix: use prepared title wrapped with a DefaultTextStyle instead of the raw title (#289)

* chore: fix typo

Closes #290

* feat: add `backgroundColor` to `MacosSheet` (#291)

* chore: fix Flutter 3.3 warnings

* fix: address ScrollController bug in MacosPopupButton (#300)

* fix(tests): account for Jan -> Dec & Dec -> Jan

date_picker_test.dart was failing due to not accounting for going from January to December and vice-versa.

* fix(plugin): Ensure the native color panel releases when closed

* Update flutter_analysis.yml

Closes #334

* Various bug fixes & minor updates (#338)

* Macos slider (#337)

* chore: run flutter format .

* chore: fix analysis

* chore: Bump version and update CHANGELOG.md

* chore: Update images to self taken ones as MacOS images are outdated

* fix: fix position offset by a small value

* fix: PR review feedback

* Update lib/src/indicators/slider.dart

---------

Co-authored-by: Reuben Turner <groovinchip@gmail.com>

* Adds `intialDate` to `MacosDatePicker` (#329)

* Adds `intialDate` to `MacosDatePicker`

* Bumps `macos_ui` version to `1.7.7`

* Apply suggestions from code review

* spelling correction

---------

Co-authored-by: Reuben Turner <groovinchip@gmail.com>

* feat: implement `MacosDisclosureButton` (#326)

* chore: update `.gitignore`

Ignores FVM and DCM

* docs(MacosApp): fix constructor docs error

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

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

* add unit test for CapacityIndicator with splits 20

* add bugfix line to CHANGELOG

* update version to 1.7.7, move change in CHANGELOG.md to version 1.7.7

* add test to check the number of filled segments of discrete CapacityIndicator

* fix warnings in copied sources mock_canvas.dart and recording_canvas.dart

* format new and changed files

* Update CHANGELOG.md

Co-authored-by: Reuben Turner <groovinchip@gmail.com>

* format indicators_page.dart, run flutter pub get to update lock files

* set dart < 3.0.0

* revert change of spec checksums in Podfile.lock

---------

Co-authored-by: Reuben Turner <groovinchip@gmail.com>

* Scrollbar overhaul & more (#342)

* feat: overhaul `MacosScrollBar`

* feat: update theme api & docs

* chore: more api cleanup

* chore: update changelog

* test: add initial test and todo's for new scrollbar

* chore: address DCM warnings

* chore: run pub upgrade

* chore: update `flutter_analysis` badge

* fix: incorrect calculation for hiding sidebar top

* fix: sidebar bottom incorrect check

* chore: update `.gitignore`

Ignores FVM and DCM

# Conflicts:
#	.gitignore

* chore: move `scrollbar.dart` into /layout

Didn't really make sense for it to be in /indicators

* feat: implement `handleHover` & `handleHoverExit`

This also removes the need for the `handleThumbPress` methods. I also did some minor renaming.

* try to fix DCM action

* trying again via pub upgrade

* Revert "try to fix DCM action"

This reverts commit 83c9fad.

* use latest DCM & ignore 2 files

* use latest DCM action

---------

Co-authored-by: Minas Giannekas <whiplashoo@users.noreply.github.com>
Co-authored-by: Jon Saw <jon.saw@gmail.com>
Co-authored-by: Minas Giannekas <whiplashoo721@gmail.com>
Co-authored-by: st merlhin <77164238+stMerlHin@users.noreply.github.com>
Co-authored-by: jtdLab <72231111+jtdLab@users.noreply.github.com>
Co-authored-by: Norbert Kozsir <norbertkozsir@gmail.com>
Co-authored-by: Elijah Luckey <luckeyelijah112@gmail.com>
Co-authored-by: Umar Salim <35866694+Umar1312@users.noreply.github.com>
Co-authored-by: Alfred Schilken <alfred@schilken.de>
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.

2 participants