Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] Top sites do not use full width in landscape mode #8788

Closed
ecsmyth opened this issue Feb 26, 2020 · 5 comments
Closed

[Bug] Top sites do not use full width in landscape mode #8788

ecsmyth opened this issue Feb 26, 2020 · 5 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:HomeScreen Feature:Shortcuts Top Sites/Topsites on the Firefox home page needs:triage Issue needs triage

Comments

@ecsmyth
Copy link

ecsmyth commented Feb 26, 2020

Steps to reproduce

  1. Add top sites until there are two+ rows in portrait mode
  2. Rotate the phone to landscape mode

Expected behavior

Full width of the screen is used to show as many top sites as fit before creating additional rows.

Actual behavior

Screenshot_20200227-022958_Firefox Nightly

Device information

  • Android device: Samsung Galaxy S10 Int'l
  • Fenix version: Fenix Nightly 200226 build 20571807

┆Issue is synchronized with this Jira Task

@ecsmyth ecsmyth added 🐞 bug Crashes, Something isn't working, .. Feature:HomeScreen Feature:Shortcuts Top Sites/Topsites on the Firefox home page labels Feb 26, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Feb 26, 2020
@gabrielluong
Copy link
Member

This seems to be a regression from #8719.

@gabrielluong
Copy link
Member

Hi @topotropic, I am wondering what we should expect when we're in landscape mode for Top Sites. Above you can see the changes that was done to force it not expand and match the parent width.

I have included screenshots from #8719 of the previous behaviour:

@topotropic
Copy link

@gabrielluong can we keep the margins and fill up the row until it's full and the break to the next row?

@mcarare
Copy link
Contributor

mcarare commented Feb 28, 2020

@gabrielluong It is not a regression from #8719, it is just the fact that GridLayoutManager is hardcoded to show 5 elements per row ( const val NUM_COLUMNS = 5 in TopSiteViewHolder )

mcarare pushed a commit to mcarare/fenix that referenced this issue Mar 9, 2020
@mcarare mcarare self-assigned this Mar 9, 2020
@ekager ekager added the eng:qa:needed QA Needed label Mar 9, 2020
@LaurentiuApahideanSV
Copy link

Verified as fixed on Firefox Preview Nightly 200310 (Build #20700607) GV: 75.0a1-20200309091841 using a Motorala G7 (Android 9) and OnePlus 6T (Android 9). When the device is in landscape mode the Top Sites are no longer limited to 5 per row but rather the length available on the screen and there is no large spacing between top sites.

@LaurentiuApahideanSV LaurentiuApahideanSV added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Mar 10, 2020
@mcarare mcarare closed this as completed Mar 10, 2020
@liuche liuche mentioned this issue Mar 12, 2020
32 tasks
severinrudie pushed a commit to severinrudie/fenix that referenced this issue Mar 18, 2020
@liuche liuche mentioned this issue Mar 24, 2020
32 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified Feature:HomeScreen Feature:Shortcuts Top Sites/Topsites on the Firefox home page needs:triage Issue needs triage
Projects
None yet
Development

No branches or pull requests

6 participants