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

Follow ups to 5626 #5638

Conversation

CrisBarreiro
Copy link
Contributor

@CrisBarreiro CrisBarreiro commented Feb 12, 2025

Task/Issue URL: https://app.asana.com/0/1205008441501016/1209378071641065

Description

Steps to test this PR

Feature 1

Feature 2

Feature 2

UI changes

Before After
!(Upload before screenshot) (Upload after screenshot)

Copy link
Contributor Author

CrisBarreiro commented Feb 12, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/follow-ups-5626 branch from aa4b887 to f1e0c8d Compare February 12, 2025 16:34
Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

Left some comments, might not be you are looking for right now so feel free to discard them.

@@ -652,7 +652,9 @@ class BrowserTabFragment :
delay(COOKIES_ANIMATION_DELAY)
}
context?.let {
omnibar.createCookiesAnimation(isCosmetic)
if ([email protected]?.maliciousSiteBlocked != true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should go via ViewModel the same way viewModel.onAutoconsentResultReceived() does

@@ -875,7 +877,7 @@ class BrowserTabFragment :
object : DefaultLifecycleObserver {
override fun onStop(owner: LifecycleOwner) {
if (isVisible) {
if (viewModel.browserViewState.value?.maliciousSiteDetected != true) {
if (viewModel.browserViewState.value?.maliciousSiteBlocked != true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -3787,7 +3789,7 @@ class BrowserTabFragment :
omnibar.cancelTrackersAnimation()
}

if (viewState.progress == MAX_PROGRESS) {
if (viewState.progress == MAX_PROGRESS && lastSeenBrowserViewState?.maliciousSiteBlocked != true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@CrisBarreiro CrisBarreiro force-pushed the feature/cris/malicious-site-protection/follow-ups-5626 branch from f1e0c8d to ab4d988 Compare February 13, 2025 16:15
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