-
Notifications
You must be signed in to change notification settings - Fork 839
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
[Android] Brave news - cards scroll fixes and tweaks #12078
Conversation
} | ||
InitBraveNewsController(); | ||
initNews(); | ||
Tab tab1 = BraveActivity.getBraveActivity().getActivityTab(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A check if (BraveActivity.getBraveActivity() != null
should probably be before that line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyZhukovsky the whole "tab1" block was useless there i think it was part of some dev logs . Same as below
@@ -644,11 +681,18 @@ protected void onDetachedFromWindow() { | |||
BraveActivity.getBraveActivity().setNewsItemsFeedCards(mNewsItemsFeedCard); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are no checks on if (BraveActivity.getBraveActivity() != null) {
but I see them in other places. Should we check it here as well?
mSettingsBar.setVisibility(View.GONE); | ||
mSettingsBar.setAlpha(0f); | ||
} | ||
Tab tab = BraveActivity.getBraveActivity().getActivityTab(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are no checks on if (BraveActivity.getBraveActivity() != null) {
but I see them in other places. Should we check it here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyZhukovsky i removed the entire "Tab" block. leftover
de8131d
to
ec715f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
@brave/legacy_qa tagging just to ease the access some of the important issues for News are here |
Verification passed on Oppo Reno 5 with Android 12 and/or Samsung Tab A with Android 10 running 1.37.55 x64 nightly build brave/brave-browser#20773 - Passed
20773.mp4brave/brave-browser#20772 - Passed
20772.mp4brave/brave-browser#20410 - Passed
20410.mp4brave/brave-browser#20771 - Failed
brave/brave-browser#20769 - Passed
20769.mp4brave/brave-browser#20768 - Passed
20768.mp4brave/brave-browser#20766 - Partial Pass or Failed
|
Resolves brave/brave-browser#20773
Resolves brave/brave-browser#20772
Resolves brave/brave-browser#20410
Resolves brave/brave-browser#20769
Resolves brave/brave-browser#20771
Resolves brave/brave-browser#20769
Resolves brave/brave-browser#20768
Resolves brave/brave-browser#20766
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: