8000 Scrollbar overhaul & more by GroovinChip · Pull Request #342 · macosui/macos_ui · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Scrollbar overhaul & more #342

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
Feb 13, 2023
Merged

Scrollbar overhaul & more #342

merged 21 commits into from
Feb 13, 2023

Conversation

GroovinChip
Copy link
Collaborator
@GroovinChip GroovinChip commented Feb 5, 2023 •
edited
Loading

This PR contains the following breaking changes:

  • MacosScrollbar has been completely overhauled and now [more closely] resembles the native macOS scrollbar in appearance and behavior. Previously, it wrapped the material scrollbar, and now creates a custom scrollbar that extends
    RawScrollbar. This resulted in the removal of several material-based properties for the scrollbar, and
    ContentArea.builder is once again a ScrollableWidgetBuilder.
  • Removes material-based scrollbar properties from MacosScrollbarThemeData

Other changes:

  • Updates dart_code_metrics and related rules
  • Moved scrollbar.dart from /indicators to /layout, as it is a more appropriate location.

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

@GroovinChip GroovinChip self-assigned this Feb 5, 2023
@GroovinChip
Copy link
Collaborator Author
8000

@whiplashoo I'd like your review on this one, if you have the time.

@GroovinChip GroovinChip changed the title Scrollbar overhaul Scrollbar overhaul & more Feb 5, 2023
@GroovinChip GroovinChip linked an issue Feb 5, 2023 that may be closed by this pull request
The example app does no longer show the bottom ListTile of the sidebar #343
Closed
whiplashoo
whiplashoo reviewed Feb 9, 2023
View reviewed changes
Copy link
Collaborator
@whiplashoo whiplashoo left a comment

Choose a reason for hiding this comment

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

The new scrollbar looks really nice, almost identical to macOS one.

My only note: shouldn't it also appear when hovering over it?

On macOS, after scrolling and while the scrollbar is visible, hovering over any point of the track, activates the scrollbar's animation and thickness effect:

Screen.Recording.2023-02-09.at.10.33.01.PM.mov

There is a handleHover method we can override in ScrollBar.

@GroovinChip
Copy link
Collaborator Author

The new scrollbar looks really nice, almost identical to macOS one.

My only note: shouldn't it also appear when hovering over it?

On macOS, after scrolling and while the scrollbar is visible, hovering over any point of the track, activates the scrollbar's animation and thickness effect:

Screen.Recording.2023-02-09.at.10.33.01.PM.mov

There is a handleHover method we can override in ScrollBar.

Thanks! Yes, the scrollbar should absolutely show the track on hover. I meant to add that but I forgot. I also don't think I knew about the handleHover override. I'll try and implement that tonight.

One thing, though, and I can't check on this now as I'm AFK, but I believe that the behavior of the scrollbar differs slightly between Apple's apps (classic).

@GroovinChip GroovinChip mentioned this pull request Feb 9, 2023
Merged
@whiplashoo
Copy link
Collaborator

Yes, sure. I can see that on Finder, the inner scrollbar doesn't have that hover effect, for example, but it's there on Safari, Notes, and Reminders 🤷‍♂️

@GroovinChip
Copy link
Collaborator Author

@whiplashoo Alright then, I'm inclined to say that the default behavior should follow how the majority of apps behave. I'll work on adding this.

As an aside, if you have any tips on testing the scrollbar, I'm all ears. I've been banging my head against the wall for like a week about that!

@GroovinChip
chore: move scrollbar.dart into /layout
d0d5b38 
Didn't really make sense for it to be in /indicators
@GroovinChip
feat: implement handleHover & handleHoverExit
80c192a 
This also removes the need for the `handleThumbPress` methods. I also did some minor renaming.
@GroovinChip
Copy link
Collaborator Author

Alrighty @whiplashoo, hover effect has been implemented!

whiplashoo
whiplashoo approved these changes Feb 10, 2023
View reviewed changes
Copy link
Collaborator
@whiplashoo whiplashoo left a comment

Choose a reason for hiding this comment

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

Yes, that looks and works great!

Can't think anything on the testing side, though. Since the scrollbar is a wrapper around RawScrollbar, I am not sure if there is a lot to test, apart from the visual stuff.

@GroovinChip
Copy link
Collaborator Author
GroovinChip commented Feb 10, 2023 •
edited
Loading

Yes, that looks and works great!

Can't think anything on the testing side, though. Since the scrollbar is a wrapper around RawScrollbar, I am not sure if there is a lot to test, apart from the visual stuff.

Amazing!

Edit: actually @whiplashoo the reason I want to test the scrollbar is I'm trying to add macos_ui to Flutter's customer testing and having tests will help alert us if something breaks RawScrollbar

Thanks so much for the review!

@GroovinChip
Copy link
Collaborator Author

The DCM check failing is driving me NUTS!!!

@GroovinChip
Revert "try to fix DCM action"
d260ecd 
This reverts commit <
8000
a class="commit-link" data-hovercard-type="commit" data-hovercard-url="https://github.com/macosui/macos_ui/commit/83c9fada5c16639b5d91f92053a6cf17a2bb57ab/hovercard" href="https://github.com/macosui/macos_ui/commit/83c9fada5c16639b5d91f92053a6cf17a2bb57ab">83c9fad.
@GroovinChip GroovinChip mentioned this pull request Feb 12, 2023
Closed
@github-actions
Copy link

Dart Code Metrics analyze report of macos_ui. ✅

Summary

  • Scanned folders: lib

  • Total scanned files: 74

  • Total lines of source code: 8922

  • Total classes: 186

  • Average Cyclomatic Number per line of code: 0.24 / 5

  • Average Source Lines of Code per method: 11

  • Total tech debt: not found

  • Found issues: 0 ✅

@GroovinChip GroovinChip merged commit e08ac84 into dev Feb 13, 2023
@GroovinChip GroovinChip deleted the scrollbar_overhaul branch February 13, 2023 15:25
GroovinChip added a commit that referenced this pull request Feb 13, 2023
@GroovinChip @whiplashoo
@jonsaw @stMerlHin @jtdLab @Norbert515 @Luckey-Elijah @Umar1312 @schilken
release: Version 1.10.0 (#355)
249cced 
* 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
Reviewers

@whiplashoo whiplashoo whiplashoo approved these changes

Assignees

@GroovinChip GroovinChip

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The example app does no longer show the bottom ListTile of the sidebar
2 participants
@GroovinChip @whiplashoo
0