Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

Fixup autofill footer in dark mode. #1115

Merged
merged 3 commits into from
Jan 9, 2020

Conversation

jhugman
Copy link
Contributor

@jhugman jhugman commented Dec 18, 2019

Fixes #1072.

This PR is a one liner which sets the background of the call to action at the bottom of the autofill dialogs.

This is both the Unlock Lockwise and Search Lockwise footers.

@jhugman jhugman requested a review from a team as a code owner December 18, 2019 12:54
@jhugman
Copy link
Contributor Author

jhugman commented Dec 18, 2019

@ddurst Please could you review this, when you have a second.

@isabelrios
Copy link
Contributor

@jhugman I just tried this branch to check the UI and saw that it is shown fine, but if you tap on the Search option the app crashes :(
Looks like that is not repro before this patch when the color was wrong :(

@abodea
Copy link
Contributor

abodea commented Dec 18, 2019

Hello, I have some more information regarding the crash issue:

The crash can be reproduced only when a website(example facebook) is opened from the entries list(from Lockwise) and tapping on the Search button.

If I open the browser manually and I tap on the Search button, the Search logins pop-up menu from Lockwise is correctly displayed and there is no crash.

I can reproduce the crash by opening a website from the entries list(lockwise) only on the latest build with the change from this PR.
Video from debug build with the changes after this PR
I tried to reproduce it on the latest build I tested 3.3.0(build5784) but I had no issues.
Video from 3.3.0(5784) no crash

Crashlog

--------- beginning of crash
2019-12-18 17:12:42.688 18419-18544/mozilla.lockbox E/AndroidRuntime: FATAL EXCEPTION: DefaultDispatcher-worker-9
    Process: mozilla.lockbox, PID: 18419
    io.reactivex.exceptions.OnErrorNotImplementedException: The exception was not handled due to missing onError handler in the subscribe() method call. Further reading: https://github.com/ReactiveX/RxJava/wiki/Error-Handling | Should be on the UI thread
        at io.reactivex.internal.functions.Functions$OnErrorMissingConsumer.accept(Functions.java:704)
        at io.reactivex.internal.functions.Functions$OnErrorMissingConsumer.accept(Functions.java:701)
        at io.reactivex.internal.observers.LambdaObserver.onError(LambdaObserver.java:77)
        at io.reactivex.internal.observers.LambdaObserver.onNext(LambdaObserver.java:67)
        at io.reactivex.internal.util.NotificationLite.accept(NotificationLite.java:246)
        at io.reactivex.subjects.BehaviorSubject$BehaviorDisposable.test(BehaviorSubject.java:569)
        at io.reactivex.subjects.BehaviorSubject$BehaviorDisposable.emitNext(BehaviorSubject.java:564)
        at io.reactivex.subjects.BehaviorSubject.onNext(BehaviorSubject.java:268)
        at mozilla.lockbox.store.ItemDetailStore$1.accept(ItemDetailStore.kt:41)
        at mozilla.lockbox.store.ItemDetailStore$1.accept(ItemDetailStore.kt:20)
        at io.reactivex.internal.observers.LambdaObserver.onNext(LambdaObserver.java:63)
        at io.reactivex.internal.operators.observable.ObservableMap$MapObserver.onNext(ObservableMap.java:62)
        at io.reactivex.internal.operators.observable.ObservableFilter$FilterObserver.onNext(ObservableFilter.java:52)
        at io.reactivex.subjects.PublishSubject$PublishDisposable.onNext(PublishSubject.java:308)
        at io.reactivex.subjects.PublishSubject.onNext(PublishSubject.java:228)
        at mozilla.lockbox.flux.Dispatcher.dispatch(Dispatcher.kt:22)
        at mozilla.lockbox.presenter.AutofillRoutePresenter$onViewReady$6.invoke(AutofillRoutePresenter.kt:77)
        at mozilla.lockbox.presenter.AutofillRoutePresenter$onViewReady$6.invoke(AutofillRoutePresenter.kt:35)
        at mozilla.lockbox.presenter.AutofillRoutePresenter$sam$io_reactivex_functions_Consumer$0.accept(Unknown Source:6)
        at io.reactivex.internal.observers.LambdaObserver.onNext(LambdaObserver.java:63)
        at io.reactivex.internal.operators.observable.ObservableMap$MapObserver.onNext(ObservableMap.java:62)
        at io.reactivex.internal.operators.observable.ObservableSwitchMap$SwitchMapObserver.drain(ObservableSwitchMap.java:297)
        at io.reactivex.internal.operators.observable.ObservableSwitchMap$SwitchMapInnerObserver.onSubscribe(ObservableSwitchMap.java:355)
        at io.reactivex.internal.operators.observable.ObservableJust.subscribeActual(ObservableJust.java:34)
        at io.reactivex.Observable.subscribe(Observable.java:12090)
        at io.reactivex.internal.operators.observable.ObservableSwitchMap$SwitchMapObserver.onNext(ObservableSwitchMap.java:127)
        at io.reactivex.internal.operators.observable.ObservableFilter$FilterObserver.onNext(ObservableFilter.java:52)
        at com.jakewharton.rxrelay2.ReplayRelay$SizeBoundReplayBuffer.replay(ReplayRelay.java:637)
        at com.jakewharton.rxrelay2.ReplayRelay.accept(ReplayRelay.java:220)
        at mozilla.lockbox.store.DataStore$unlockInternal$4.invoke(DataStore.kt:305)
        at mozilla.lockbox.store.DataStore$unlockInternal$4.invoke(DataStore.kt:39)
        at mozilla.lockbox.store.DataStore$sam$io_reactivex_functions_Consumer$0.accept(Unknown Source:6)
        at io.reactivex.internal.observers.LambdaObserver.onNext(LambdaObserver.java:63)
        at io.reactivex.internal.operators.observable.ObservableMap$MapObserver.onNext(ObservableMap.java:62)
        at io.reactivex.internal.operators.observable.ObservableTake$TakeObserver.onNext(ObservableTake.java:64)
        at io.reactivex.internal.operators.observable.ObservableSkip$SkipObserver.onNext(ObservableSkip.java:56)
        at io.reactivex.internal.operators.observable.ObservableDoOnEach$DoOnEachObserver.onNext(ObservableDoOnEach.java:101)
        at io.reactivex.internal.operators.observable.ObservableSwitchMap$SwitchMapObserver.drain(ObservableSwitchMap.java:297)
2019-12-18 17:12:42.689 18419-18544/mozilla.lockbox E/AndroidRuntime:     at io.reactivex.internal.operators.observable.ObservableSwitchMap$SwitchMapInnerObserver.onNext(ObservableSwitchMap.java:374)
        at com.jakewharton.rxrelay2.BehaviorRelay$BehaviorDisposable.test(BehaviorRelay.java:354)
        at com.jakewharton.rxrelay2.BehaviorRelay$BehaviorDisposable.emitNext(BehaviorRelay.java:348)
        at com.jakewharton.rxrelay2.BehaviorRelay.accept(BehaviorRelay.java:127)
        at mozilla.lockbox.store.DataStore$updateList$1.accept(DataStore.kt:384)
        at mozilla.lockbox.store.DataStore$updateList$1.accept(DataStore.kt:39)
        at io.reactivex.internal.observers.ConsumerSingleObserver.onSuccess(ConsumerSingleObserver.java:62)
        at io.reactivex.internal.operators.single.SingleCreate$Emitter.onSuccess(SingleCreate.java:67)
        at kotlinx.coroutines.rx2.RxSingleCoroutine.onCompleted(RxSingle.kt:45)
        at kotlinx.coroutines.AbstractCoroutine.onCompletionInternal(AbstractCoroutine.kt:102)
        at kotlinx.coroutines.JobSupport.tryFinalizeSimpleState(JobSupport.kt:275)
        at kotlinx.coroutines.JobSupport.tryMakeCompleting(JobSupport.kt:807)
        at kotlinx.coroutines.JobSupport.makeCompletingOnce$kotlinx_coroutines_core(JobSupport.kt:787)
        at kotlinx.coroutines.AbstractCoroutine.resumeWith(AbstractCoroutine.kt:111)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:46)
        at kotlinx.coroutines.DispatchedTask.run(Dispatched.kt:241)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:594)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.access$runSafely(CoroutineScheduler.kt:60)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:740)
     Caused by: java.lang.AssertionError: Should be on the UI thread
        at mozilla.lockbox.support.DebugSupportKt.assertOnUiThread(DebugSupport.kt:17)
        at mozilla.lockbox.support.DebugSupportKt.assertOnUiThread$default(DebugSupport.kt:15)
        at mozilla.lockbox.view.ItemDetailFragment.setPasswordVisible(ItemDetailFragment.kt:98)
        at mozilla.lockbox.presenter.ItemDetailPresenter$onViewReady$12.accept(ItemDetailPresenter.kt:145)
        at mozilla.lockbox.presenter.ItemDetailPresenter$onViewReady$12.accept(ItemDetailPresenter.kt:54)
        at io.reactivex.internal.observers.LambdaObserver.onNext(LambdaObserver.java:63)
        	... 53 more

Copy link
Contributor

@eliserichards eliserichards left a comment

Choose a reason for hiding this comment

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

Small request to avoid the overdraw error.

@@ -12,6 +12,7 @@
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:theme="@style/AppTheme"
android:background="@color/background_white"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the right place to define the background since it creates the overdraw error. I would prefer it to be set in the styles or explicitly set when the autofill dialog is created.

@jhugman jhugman force-pushed the jhugman/1072-fixup-autofill-footer-darkmode branch from eeed1f5 to 53abf37 Compare January 8, 2020 16:10
Copy link
Contributor

@eliserichards eliserichards left a comment

Choose a reason for hiding this comment

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

Fantastic. Shipit! :shipit:

@@ -138,10 +138,12 @@ class DisplayItemPresenter(
.addTo(compositeDisposable)

networkStore.isConnected
.observeOn(mainThread())
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

@@ -11,8 +11,8 @@
xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:theme="@style/AppTheme"
>
style="@style/AutofillRemoteViewSyle">
Copy link
Contributor

Choose a reason for hiding this comment

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

👏 🎉 🚀

@jhugman jhugman merged commit 28f4633 into master Jan 9, 2020
@jhugman jhugman deleted the jhugman/1072-fixup-autofill-footer-darkmode branch January 9, 2020 15:11
@ddurst ddurst mentioned this pull request Jan 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search Firefox Lockwise bottom button color from autofill screen is dark
4 participants