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

New search window with library #3225

Closed
wants to merge 8 commits into from

Conversation

EmmanuelMess
Copy link
Member

Description

Issue tracker

Automatic tests

  • Added test cases

Manual tests

  • Done

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembledebug
  • ./gradlew spotlessCheck

@EmmanuelMess EmmanuelMess force-pushed the emmanuelmess/fix/search branch from 0569817 to f8dbfbb Compare March 18, 2022 00:52
@EmmanuelMess EmmanuelMess marked this pull request as ready for review March 21, 2022 03:10
VishnuSanal
VishnuSanal previously approved these changes Mar 23, 2022
Copy link
Member

@VishnuSanal VishnuSanal 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 to me 🚀

@VishalNehra
Copy link
Member

VishalNehra commented Mar 26, 2022

Issues:

  1. Orientation of hint text doesn't seem right. Previous one was inline with other content below it.
  2. it doesn't indicate that it's currently selected, normally edittext has an underline when we're typing in it.
  3. Also there's a strange black border around the whole view when we're typing.
  4. Heigh of whole bar seem to be smaller than previous one.

Please refer to screenshots for comparison.




@EmmanuelMess
Copy link
Member Author

@VishalNehra the bar things is weird, I'll try to replicate, otherwise it is up to Material Design standards: see https://material.io/design/navigation/search.html#persistent-search.

@EmmanuelMess EmmanuelMess marked this pull request as draft March 31, 2022 05:03
@EmmanuelMess EmmanuelMess force-pushed the emmanuelmess/fix/search branch 3 times, most recently from 8fa5f3c to 5b4d40f Compare March 31, 2022 20:28
@EmmanuelMess
Copy link
Member Author

@TranceLove I am not quite sure what broke here, any ideas?

@TranceLove
Copy link
Collaborator

@TranceLove I am not quite sure what broke here, any ideas?

Sorry, something wrong in test code. See #3243

@EmmanuelMess EmmanuelMess force-pushed the emmanuelmess/fix/search branch from 5b4d40f to 5b56353 Compare April 1, 2022 21:51
@EmmanuelMess EmmanuelMess marked this pull request as ready for review April 1, 2022 22:23
@EmmanuelMess EmmanuelMess requested a review from VishnuSanal April 1, 2022 22:24
@EmmanuelMess
Copy link
Member Author

Any news?

@VishalNehra
Copy link
Member

Will check today.

@VishalNehra
Copy link
Member

Getting following crash when i rotate my screem

Issue explanation (write below this line)

Exception

  • App Name: Amaze File Manager
  • Package: com.amaze.filemanager.debug
  • Version: 3.6.7
  • User Action: UI Error
  • Request: Application crash
  • OS: Linux OnePlus/OnePlus9R_IND/OnePlus9R:11/RKQ1.201112.002/2201212022:user/release-keys 11 - 30
  • Device: OnePlus9R
  • Model: LE2101
  • Product: OnePlus9R_IND
Crash log

java.lang.NullPointerException: Attempt to invoke virtual method 'void android.view.View.getLocationOnScreen(int[])' on a null object reference
	at com.amaze.filemanager.ui.views.appbar.SearchView.hideSearchView(SearchView.kt:120)
	at com.amaze.filemanager.ui.views.appbar.SearchView$3.onFocusCleared(SearchView.kt:67)
	at xyz.quaver.floatingsearchview.FloatingSearchView.setSearchFocused(FloatingSearchView.kt:196)
	at xyz.quaver.floatingsearchview.FloatingSearchView.onRestoreInstanceState(FloatingSearchView.kt:811)
	at android.view.View.dispatchRestoreInstanceState(View.java:20890)
	at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:4011)
	at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:4017)
	at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:4017)
	at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:4017)
	at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:4017)
	at android.view.ViewGroup.dispatchRestoreInstanceState(ViewGroup.java:4017)
	at android.view.View.restoreHierarchyState(View.java:20868)
	at com.android.internal.policy.PhoneWindow.restoreHierarchyState(PhoneWindow.java:2204)
	at android.app.Activity.onRestoreInstanceState(Activity.java:1679)
	at android.app.Activity.performRestoreInstanceState(Activity.java:1632)
	at android.app.Instrumentation.callActivityOnRestoreInstanceState(Instrumentation.java:1383)
	at android.app.ActivityThread.handleStartActivity(ActivityThread.java:3664)
	at android.app.servertransaction.TransactionExecutor.performLifecycleSequence(TransactionExecutor.java:221)
	at android.app.servertransaction.TransactionExecutor.cycleToPath(TransactionExecutor.java:201)
	at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:173)
	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:97)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2257)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loop(Looper.java:233)
	at android.app.ActivityThread.main(ActivityThread.java:8030)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:631)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978)


@VishalNehra
Copy link
Member

Rejecting this due to new search ui in #3730

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.

4 participants