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

[Bug] There is no limit of how many top-sites you can add #8312

Closed
abodea opened this issue Feb 11, 2020 · 47 comments · Fixed by #14116
Closed

[Bug] There is no limit of how many top-sites you can add #8312

abodea opened this issue Feb 11, 2020 · 47 comments · Fixed by #14116
Assignees
Labels
E5 Estimation Point: about 5 days eng:qa:verified QA Verified Feature:Shortcuts Top Sites/Topsites on the Firefox home page S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist

Comments

@abodea
Copy link
Member

abodea commented Feb 11, 2020

Update from UX:

  • To keep the design scalable, let's limit top sites to 16 (2 x 8 swipe-able, see screenshot and please ignore the pin icon on the first three tabs for now)
  • Once people add their 9th top site we show dots below top sites to indicate that there's more
  • People can swipe (similar to how it works in Fennec) to see their 9th-16th top site
  • When people are about to add a 17th top site a dialog is displayed telling them that they reached the max amount for top sites

image


Original request:

Steps to reproduce

  1. Launch Fenix.
  2. Add 50+ top-sites.

Expected behavior

There should be a limit regarding how many top-sites you can add.

Actual behavior

50+ top-sites can be added.

Device information

  • Android device: Samsung Galaxy S10+(Android 9)
  • Fenix version: Latest Nightly from 2/11.

20200211-175517

┆Issue is synchronized with this Jira Task

@abodea abodea added 🐞 bug Crashes, Something isn't working, .. S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist Feature:Shortcuts Top Sites/Topsites on the Firefox home page labels Feb 11, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Feb 11, 2020
@mcarare mcarare self-assigned this Mar 20, 2020
@mcarare mcarare added the needs:UX-feedback Needs UX Feedback label Mar 30, 2020
@mcarare mcarare removed their assignment Mar 30, 2020
@brampitoyo
Copy link

Paging @topotropic for feedback. Should there be a limit to how many items users can add under Top Sites?

@mcarare just to be aware of the fact that most users will probably not create enough Top Sites to ever see this problem.

@AmyYLee
Copy link
Collaborator

AmyYLee commented Jun 3, 2020

@topotropic to follow-up

@topotropic
Copy link

Let's limit to 8 top sites (2 rows a 4 sites) – when people add the 9th top site we remove the oldest.

@topotropic topotropic removed the needs:UX-feedback Needs UX Feedback label Jun 4, 2020
@topotropic topotropic removed their assignment Jun 4, 2020
@cadeyrn
Copy link
Contributor

cadeyrn commented Jun 4, 2020

@topotropic:

I already saw a screenshot of a user with more top sites. Why should this be limited at all? Sure, it doesn't look great but it's the user's decision, no? I don't need top sites at all but if it's useful for someone to have 20 top sites or even more why should this not be allowed? Is there a technical reason why it should be limited? Unfortunately there is no explanation in this issue what the benefit of such a limit is.

Please also note that with the new tabs tray and the removal of the tabs from the start screen there is more room that could be used for top sites now - when the user wants it.

Also I am not convinved that it's a good idea to remove the oldest top site when trying to add more top sites than allowed. If I were to use the top sites feature, I would probably add the most important website at first. But with this proposal the website I consider as the most important will be the first website that get removed again.

@liuche
Copy link
Contributor

liuche commented Jun 4, 2020

We want to keep our design scalable for other things we want to add to the homescreen (synced tabs, bookmarks, etc)

Should be a non-destructive method.

@liuche
Copy link
Contributor

liuche commented Jun 4, 2020

for release minimum, we need to have a way to cap this before release, but can improve it in subsequent releases.

@betsymi
Copy link

betsymi commented Jun 12, 2020

Strings for the dialog when top site limit is reached.

Top site limit reached

To add a new top site, remove one. Touch and hold the site and select remove.

OK, Got It

@cadeyrn
Copy link
Contributor

cadeyrn commented Aug 26, 2020

is simply trash!

https://www.mozilla.org/en-US/about/governance/policies/participation/:

Be Respectful

Value each other’s ideas, styles and viewpoints. We may not always agree, but disagreement is no excuse for poor manners. Be open to different possibilities and to being wrong. Be respectful in all interactions and communications, especially when debating the merits of different options.

@sheikh-azharuddin
Copy link

Uff sorry @cadeyrn Mozilla fanboy.... Do Mozilla listens..or they simply do what they think is right! .

@klint
Copy link

klint commented Aug 26, 2020

@cadeyrn : you said that the current limit is 2 slides? But I had far more than 16 top sites before this was implemented and now I have 5 slides which I can swipe to. The only issue is that there are only 2 progress dots instead of 5.
Édit: all that being said, why is there a limit on the the number of slides here? Apart from the wrong number of dots, it is quite usable with more than 2 slides.

@klint
Copy link

klint commented Aug 26, 2020

Another comment on the swipe action itself : maybe because it is an horizontal swipe on a narrow band at the top of the screen, I find it rather uneasy to swipe. It often ends up in a diagonal gesture that actually scrolls the page down instead of scrolling through the slides...

@cadeyrn
Copy link
Contributor

cadeyrn commented Aug 26, 2020

Uff sorry @cadeyrn Mozilla fanboy....

I am a "fanboy" because I asked you not to be disrepectful. Wow, against such a logic it's hard to argue… Really, it's not okay how you interact with other people! 😐

you said that the current limit is 2 slides? But I had far more than 16 top sites before this was implemented and now I have 5 slides which I can swipe to.

Interesting. Maybe I misunderstood the code? @gabrielluong should be able to clarify. 😊

@gabrielluong
Copy link
Member

Currently, it is indeed designed to be a max of 16 top sites displayed in 4 x 2 grid. We can revisit the max of 16 top sites. I imagine the rationale for the 4 x 2 grid and paging is to preserve the vertical space of the home screen. This is an active development space, so expect continual improvements.

@klint
Copy link

klint commented Aug 27, 2020

you said that the current limit is 2 slides? But I had far more than 16 top sites before this was implemented and now I have 5 slides which I can swipe to.

Interesting. Maybe I misunderstood the code? @gabrielluong should be able to clarify. 😊
@cadeyrn It seems the code was changed yesterday as I'm able to swipe through 2 slides only, now. Which I find quite sad, as I don't really understand why the slides should be limited to 2 (and 16 top sites), after being able to play with more yesterday :)

@klint
Copy link

klint commented Aug 27, 2020

Another comment on the swipe action itself : maybe because it is an horizontal swipe on a narrow band at the top of the screen, I find it rather uneasy to swipe. It often ends up in a diagonal gesture that actually scrolls the page down instead of scrolling through the slides...

FYI, I have opened a new issue #14287 that is somehow related to my remark above

@lazymonkey2
Copy link

I don't agree on limiting top sites: I use many and don't want any limit, it's my decision how much top sites I should see.
However I agree about adding "dots below top sites to indicate that there's more", once reached a limit (maybe 2 rows?).
But ONLY if fenix remembers the user choice about showing all top sites or showing less rows and the dots.
If the user want to see all top sites he just taps on the dots, and fenix will always show all top sites, without limits.

@OUTDOOOR
Copy link

User customisation of rows and columns could be very useful. Current solution with two sides of the Top Pages is not comfortable and is much worse than previous version. Sliding between two sides is really annoying. I hope, that in next updates it will be improved.

@LaurentiuApahideanSV
Copy link

I have tested the issue on Firefox Preview Nightly 200831 (Build #2015761139). Only 16 websites can be added to topsites, however when trying to add the 17th top site there is no dialog informing the user that they reached the max amount of top sites.

Devices used:

  • OnePlus 6T (Android 9)
  • Huawei MediaPad M3 (Android 7.0)

@gabrielluong
Copy link
Member

I have tested the issue on Firefox Preview Nightly 200831 (Build #2015761139). Only 16 websites can be added to topsites, however when trying to add the 17th top site there is no dialog informing the user that they reached the max amount of top sites.

Devices used:

  • OnePlus 6T (Android 9)
  • Huawei MediaPad M3 (Android 7.0)

@LaurentiuApahideanSV Thanks for the catch! I have filed #14529 for the dialog. This wasn't a release blocker for this feature so I didn't end up implementing it just yet.

@gabrielluong
Copy link
Member

gabrielluong commented Aug 31, 2020

@LaurentiuApahideanSV Given the above, should we say this is qa-verified? I am also curious about the qa on the add/remove top sites. The visited sites should display "Delete from history" and also delete the item from the History.

@sv-ohorvath
Copy link
Contributor

@gabrielluong I just tried this and without that limitation, users can re-try adding a top site 10 times until they give up. Doing so, will create a queue of 10 identical items to add to the top sites list, and once you remove a top site it gets added. If you have the same website 10x times in that list, you will see it and have to remove it 10 times to get rid of it.

@LaurentiuApahideanSV
Copy link

@gabrielluong Sites present on the Top Sites section do have an option to remove them but two issue have been encountered regarding this: #14538 and #11188. I will mark this issue as verfied as all other encountered issues have been reported.

@madb1lly
Copy link

madb1lly commented Sep 1, 2020

Hi all,

The new larger icons introduced today (tonight? 😉) in Nightly are much better, #13765. However, On my Nokia 8 it's still not a good use of space, I could comfortably fit 6 icons per row.

Maybe if the spacing between icons in rows and columns was the same it would be more aesthetically pleasing too, because it would be a grid of squares rather than a grid of rectangles.

Cheers 🙂

@gabrielluong
Copy link
Member

Closing this issue now that it is eng:qa:verified

@LaurentiuApahideanSV
Copy link

Verified as fixed on Firefox Preview Beta 81.1.0-beta.2 (Build #2015761657).

Devices used:

  • Huawei MediaPad M3 (Android 7.0)
  • OnePlus 6T (Android 9)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
E5 Estimation Point: about 5 days eng:qa:verified QA Verified Feature:Shortcuts Top Sites/Topsites on the Firefox home page S2 Major Functionality/product severely impaired and a satisfactory workaround doesn't exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.