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

feat: implement MacosDisclosureButton #326

Merged
merged 5 commits into from
Feb 4, 2023

Conversation

Umar1312
Copy link
Contributor

@Umar1312 Umar1312 commented Jan 9, 2023

Pre-launch Checklist

  • I have run dartfmt on all changed files
  • I have incremented the package version as appropriate and updated CHANGELOG.md with my changes
  • I have added/updated relevant documentation
  • I have run "optimize/organize imports" on all changed files
  • I have addressed all analyzer warnings as best I could

@Umar1312 Umar1312 force-pushed the feature/disclosure_button branch from 32c1c6e to 77dff30 Compare January 9, 2023 14:58
@Umar1312 Umar1312 mentioned this pull request Jan 11, 2023
10 tasks
@GroovinChip
Copy link
Collaborator

Hi @Umar1312, thanks for opening this PR!

Please address the failing check, and then I can review.

@Umar1312 Umar1312 force-pushed the feature/disclosure_button branch from 0899ff3 to 12b4bbe Compare January 14, 2023 09:52
@Umar1312
Copy link
Contributor Author

Hi @Umar1312, thanks for opening this PR!

Please address the failing check, and then I can review.

Addressed. Please check.

Thanks!

@GroovinChip
Copy link
Collaborator

@Umar1312 The Flutter analysis check is still failing, looks like your test file.

@GroovinChip
Copy link
Collaborator

Also, if you could please upload a gif of the widget in action, I'd really appreciate it. I don't generally have the time to pull PR branches and manually run the example to test the new code, so gifs are essential to my reviews.

@Umar1312 Umar1312 force-pushed the feature/disclosure_button branch from 12b4bbe to 516d57d Compare January 14, 2023 18:55
@Umar1312
Copy link
Contributor Author

Addressed. Also PFA the gif of widget in action.

Thanks!

DisclosureButtonGIf

@GroovinChip GroovinChip changed the title feat: implement MacosDisclosureButton feat: implement MacosDisclosureButton Jan 24, 2023
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 so I've done a more thorough review and there are some fundamental changes that should be made:

  • The disclosure control that is used for showing and hiding information and functionality associated with a view or list of items is called a Disclosure triangle. The widget should be updated to reflect this.
  • The disclosure control that is used for showing and hiding functionality associated with a specific control is called a Disclosure button. This one looks like what you've made, but the chevron points down.

I think for both of these modes, the widget should operate in a similar manner to Flutter's ExpansionTile, which can show/hide other widgets.

lib/src/buttons/disclosure_button.dart Outdated Show resolved Hide resolved
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.

Thought I'd done this as part of my previous comment but I guess not. Please see that and make the required changes. Thanks!

@Umar1312
Copy link
Contributor Author

@GroovinChip Thanks for reviewing. I'm not quite sure on what changes I should make for this.

  1. Going through the Apple's Docs, what we are trying to to implement is NSBezelStyleRoundedDisclosure. The actual implementation of the hide/show part should be the responsibility of the user as is the case for Apple's own implementation.

  2. If you still want to incorporate the show/hide functionality then we should probably not view it as a button widget, as it's scope has now changed considerably.

Let me know what you think about this.

@GroovinChip
Copy link
Collaborator

@Umar1312 I'll think about this and get back to you, thanks.

@GroovinChip
Copy link
Collaborator

GroovinChip commented Feb 3, 2023

OK @Umar1312 we can keep things as they are. Before I review, please update your branch with the latest updates from dev.

@Umar1312 Umar1312 requested a review from GroovinChip February 3, 2023 18:25
@Umar1312
Copy link
Contributor Author

Umar1312 commented Feb 3, 2023

@GroovinChip Thanks, updated.

@GroovinChip
Copy link
Collaborator

GroovinChip commented Feb 4, 2023

@Umar1312 I actually just overhauled MacosScrollBar which allows the return of the ScrollController to ContentArea. If you want I can land that first and you can revert 2fa9902, or we can land this one and I'll fix it in the PR to land the changes to MacosScrollBar. What would you like to do?

EDIT: Let's actually land this one first, don't worry about it :)

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 some minor documentation requests :)

lib/src/buttons/disclosure_button.dart Show resolved Hide resolved
lib/src/buttons/disclosure_button.dart Show resolved Hide resolved
lib/src/buttons/disclosure_button.dart Outdated Show resolved Hide resolved
@Umar1312
Copy link
Contributor Author

Umar1312 commented Feb 4, 2023

@GroovinChip Updated.

@Umar1312 Umar1312 requested a review from GroovinChip February 4, 2023 08:36
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 7979172 into macosui:dev Feb 4, 2023
@Umar1312 Umar1312 deleted the feature/disclosure_button branch February 5, 2023 11:35
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