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

Cleanup #523

Merged
merged 6 commits into from
Sep 24, 2023
Merged

Cleanup #523

merged 6 commits into from
Sep 24, 2023

Conversation

clragon
Copy link
Collaborator

@clragon clragon commented Sep 15, 2023

This PR includes:

  • applied formatting everywhere
  • applied recommended flutter lints everywhere
  • regenerated example
  • applied more consistent code style

For future PRs to be considered:

  • supportedPlatform seems outdated / incorrect. the latest flutter_keyboard_visibility supports all platforms except web.
  • the code uses alot of unnecessary this.
  • the hardcoded duration for dialogs might be resolvable by listening to the animation on the dialog route
  • some public members are missing doc comments (enableInteractiveSelection, autofillHints)
  • some widgets use static colors where it would be more appropriate to pull colors from the surrounding ThemeData

@clragon
Copy link
Collaborator Author

clragon commented Sep 15, 2023

To resolve merge conflicts after this PR has been merged:

  • open your branch
  • git reset --soft <commit hash of the last commit before yours>
  • stash all your changes
  • git pull
  • git reset --hard master
  • unstash all your changes
  • dart format .
  • dart fix --apply
  • double check formatting of your changed files
  • commit and git push --force

note that these are destructive and powerful git commands.
please be careful with any unsaved work.
You can use "apply" instead of "pop" for git stashes to retain them in case something goes wrong.

@sjmcdowall sjmcdowall merged commit 4d67537 into AbdulRahmanAlHamali:master Sep 24, 2023
@clragon clragon deleted the cleanup branch September 24, 2023 13:22
@sjmcdowall
Copy link
Collaborator

@clragon -- I merged the code but after upgrading to flutter 3.15.3 (or whatever) 2 tests fail so I am not going to publish this yet ... can you submit fixes for the tests -- here is the output ...

…/master/flutter_typeahead  c6ff9b2  master ⍟4  flutter test  1 ↵  10013  16:53:01
00:03 +1: /Users/sjm/play/master/flutter_typeahead/test/cupertino_typeahead_test.dart: Cupertino TypeAhead widget tests Initial UI Test
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: exactly 6 matching nodes in the widget tree
Actual: _WidgetTypeFinder:<5 widgets with type "CupertinoTypeAheadFormField" (ignoring
offstage widgets): CupertinoTypeAheadFormField(dependencies: [UnmanagedRestorationScope],
state: CupertinoTypeAheadFormFieldState#50797),
CupertinoTypeAheadFormField(dependencies: [UnmanagedRestorationScope], state:
CupertinoTypeAheadFormFieldState#b9d1c), CupertinoTypeAheadFormField(dependencies:
[UnmanagedRestorationScope], state: CupertinoTypeAheadFormFieldState#95302), ...>
Which: is not enough

When the exception was thrown, this was the stack:
#4 main.. (file:///Users/sjm/play/master/flutter_typeahead/test/cupertino_typeahead_test.dart:15:7)


(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
file:///Users/sjm/play/master/flutter_typeahead/test/cupertino_typeahead_test.dart line 15
The test description was:
Initial UI Test
════════════════════════════════════════════════════════════════════════════════════════════════════
00:03 +1 -1: /Users/sjm/play/master/flutter_typeahead/test/cupertino_typeahead_test.dart: Cupertino TypeAhead widget tests Initial UI Test [E]
Test failed. See exception logs above.
The test description was: Initial UI Test

To run this test again: /Users/sjm/Development/tools/flutter/bin/cache/dart-sdk/bin/dart test /Users/sjm/play/master/flutter_typeahead/test/cupertino_typeahead_test.dart -p vm --plain-name 'Cupertino TypeAhead widget tests Initial UI Test'
00:03 +1 -1: /Users/sjm/play/master/flutter_typeahead/test/material_typeahead_test.dart: Material TypeAhead widget tests Initial UI Test
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following TestFailure was thrown running a test:
Expected: exactly 6 matching nodes in the widget tree
Actual: _WidgetTypeFinder:<4 widgets with type "TypeAheadFormField" (ignoring offstage
widgets): TypeAheadFormField(dependencies: [UnmanagedRestorationScope], state:
_TypeAheadFormFieldState#a9836), TypeAheadFormField(dependencies:
[UnmanagedRestorationScope], state: _TypeAheadFormFieldState#264e9),
TypeAheadFormField(dependencies: [UnmanagedRestorationScope], state:
_TypeAheadFormFieldState#8219f), ...>
Which: is not enough

When the exception was thrown, this was the stack:
#4 main.. (file:///Users/sjm/play/master/flutter_typeahead/test/material_typeahead_test.dart:15:7)


(elided one frame from package:stack_trace)

This was caught by the test expectation on the following line:
file:///Users/sjm/play/master/flutter_typeahead/test/material_typeahead_test.dart line 15
The test description was:
Initial UI Test
════════════════════════════════════════════════════════════════════════════════════════════════════
00:03 +1 -2: /Users/sjm/play/master/flutter_typeahead/test/material_typeahead_test.dart: Material TypeAhead widget tests Initial UI Test [E]
Test failed. See exception logs above.
The test description was: Initial UI Test

To run this test again: /Users/sjm/Development/tools/flutter/bin/cache/dart-sdk/bin/dart test /Users/sjm/play/master/flutter_typeahead/test/material_typeahead_test.dart -p vm --plain-name 'Material TypeAhead widget tests Initial UI Test'
00:05 +20 -2: Some tests failed.

@clragon
Copy link
Collaborator Author

clragon commented Sep 24, 2023

certainly, I will take a look.

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