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

fix bottom padding for Top Sites #30674

Closed
kjozwiak opened this issue May 30, 2023 · 5 comments · Fixed by brave/brave-core#18708
Closed

fix bottom padding for Top Sites #30674

kjozwiak opened this issue May 30, 2023 · 5 comments · Fixed by brave/brave-core#18708

Comments

@kjozwiak
Copy link
Member

Description

We'll need to fix/improve the padding on Top Sites, specifically at the bottom padding. You'll notice that the padding at the top looks good but there's not enough padding at the bottom. See example below.

Steps to reproduce

  1. install brave and open a new tab (Top Tiles don't load on the first initial NTP)
  2. once you've opened a NTP, you'll notice that there isn't enough bottom padding for Top Sites

Actual result

Screenshot_20230529-154644

Expected result

We should add more padding at the bottom of the Top Sites so it looks the same as the top padding. Right now, it doesn't look symmetrical.

Issue reproduces how often

100% reproducible using the STR/Cases outlined above.

Version/Channel Information:

  • Can you reproduce this issue with the current Play Store version? Yes
  • Can you reproduce this issue with the current Play Store Beta version? Yes
  • Can you reproduce this issue with the current Play Store Nightly version? Yes

Device details

  • Install type (ARM, x86): ARM
  • Device type (Phone, Tablet, Phablet): Pixel 6
  • Android version: Android 14 (also reproducible with Android 13)

Brave version

Brave | 1.54.18 Chromium: 114.0.5735.53 (Official Build) canary (32-bit)
--- | ---
Revision | c499d7ea22c8b2dba278465a5df7b86a8efa4e64-refs/branch-heads/5735@{#970}
OS | Android 13; Build/UPB2.230407.014; 33; UpsideDownCake

Website problems only

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Additional information

@mlntr
Copy link

mlntr commented May 30, 2023

This fix can be implemented while doing #30668 👍

@kjozwiak
Copy link
Member Author

kjozwiak commented Jun 8, 2023

The above requires 1.53.86 or higher for 1.53.x verification 👍

@hffvld
Copy link
Contributor

hffvld commented Jun 22, 2023

Verified on Pixel 7 and Galaxy Tab S8 using version(s):

Device/OS: 
- Pixel 7 [panther_beta-user 14 UPB3.230519.008 release-keys]
- Galaxy Tab S8 [gts8wifixx-user 13 TP1A.220624.014 release-keys]
Brave build: 1.53.102
Chromium: 114.0.5735.133 (Official Build) (64-bit)
Revision: fbfa2ce68d01b2201d8c667c2e73f648a61c4f4a-refs/branch-heads/5735@{#1270}

STEPS:

  1. Launch Brave
  2. Open NTP > Verify

ACTUAL RESULTS:

  • Verified that padding at the bottom and at the top of the Top Sites looks the same (symmetrical).
Phone. Portrait Mode Phone. Landscape Mode
1 2
Tablet Portrait Mode Tablet. Landscape Mode
1 2

@kjozwiak
Copy link
Member Author

kjozwiak commented Jul 6, 2023

The above requires 1.52.130 or higher for 1.52.x verification 👍 Removing the QA Pass label as the above needs to be rechecked with 1.52.x.

@Uni-verse
Copy link
Contributor

Verified on Samsung Galaxy S21 5G using version:

Brave	1.52.130 Chromium: 114.0.5735.198 (Official Build) (32-bit) 
Revision	c3029382d11c5f499e4fc317353a43d411a5ce1c-refs/branch-heads/5735@{#1394}
OS	Android 13; Build/TP1A.220624.014; 33; REL
  • Ensured that bottom padding is adjusted for Top Sites on NTP
Old New New New (Landscape)
screenshot-1688659187610 screenshot-1688659267041 screenshot-1688659075878 screenshot-1688659279932

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants