-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[Desktop] Top sites on New Tab Page are missing #11500
Comments
Seeing this as well in 1 out of 3 desktop linux machines. Top sites, even toggled on and off have disappeared. |
cc: @simonhong |
@MikeScotty The fix for #9929 is landed in stable(1.13). |
We definitely have some work to do on new tab page I think the right solution here might be to match the logic Chromium has:
We might be able to get all of this logic for free. You can demo yourself by visiting @rebron @karenkliu WDYT? 😄 |
@bsclifton Yep - this is pretty much what we had planned in top sites spec and it's just been waiting to be implemented. I agree with most of what you said, except these two points:
In our spec, we've defined that the default top sites behavior is shown by frecency until the user indicates they would like to customize their shortcuts themselves instead of having it be shown automatically by Brave (this is the way it works in Chrome as well). While the top sites are shown by frecency, there is a maximum of 6 tiles displayed. But no matter how many displayed, the Along with that user action, these are the other actions that causes the top sites behavior to switch from frecency to user-curated:
|
Working through a proof of concept on this. I currently have the NTP showing the same results as Chromium (deleting all of the bug ridden custom code we have for maintaining this list and allowing pinning, etc). Fully matching the Chromium behavior should be quite easy and would be a nice cleanup (will fix a LOT of bugs) Very early preview work in progress branch viewable here: Will continue to check this out. Thanks for your patience folks 😄 |
If I finish the work on this branch (above), it should fix:
and then it makes future work easier
|
So you're saying that in the next version of brave will those problems fixed? |
@MikeScotty I can't promise a version, but I am working through this and will try to finish ASAP |
@bsclifton ok |
- Move gridSiteState to sessionStorage (not localStorage) and default value for "use custom links". Includes cleanup of previous localStorage value (if user has one). sessionStorage seems to be persisted between tabs and should be cleaned up when browser is closed. - X to remove tiles is now right aligned - Update top sites mode/visible setting when NTP gets updates - Fixed disabled icon showing when tiles are movable (condition was reversed) - Title case the new preference (to match existing preference) And of course, closing keywords for this PR: Fixes brave/brave-browser#11500 Fixes brave/brave-browser#9788 Fixes brave/brave-browser#9457 Fixes brave/brave-browser#11551
Verification passed on
Verified passed with
Verified test plan from brave/brave-core#6584 |
Has this actually been fixed? |
@SXMacdara Nope. The problem is still there. |
@MikeScotty why is this closed if the issue is not fixed? |
@SXMacdara I have no idea. Maybe because it's been added to the to-do list? |
@MikeScotty this will roll out with the 1.17 release which is due to be released today. It's already fixed in Beta and Nightly |
Excellent
…On Thu, Nov 19, 2020 at 10:42 AM Brian Clifton ***@***.***> wrote:
@MikeScotty <https://github.com/MikeScotty> this will roll out with the
1.17 release which is due to be released today. It's already fixed in Beta
and Nightly
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11500 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARXAIANVRYPS35HZF7L5TP3SQVRKJANCNFSM4QOODNEA>
.
|
I have version 1.17.73 and the problem is STILL occurring. JSYK I'm using linux mint 20. |
@bsclifton I am hoping you are the right person to address about this. I guess I am unsure as to why this is closed. I have the latest version of Brave V1.17.73. Top Sites is not fixed. In this latest version you can't even pin or rearrange your Top Sites anymore. Whatever was done in the way of improvement didn't work. At also appears as if the reviewer above has stated that from their perspective/experience this has not been fixed on their end either. |
@SXMacdara the not being able to move will be addressed with #7493, which also will allow for adding your own top sites. This is the next work I'll be looking at. Are you experiencing a problem with sites missing? @MikeScotty I can create a new issue for the problem you're experiencing. After updating to 1.17.73, you're seeing the top sites shown on new tab page be removed every time you quit/relaunch? If this is the case, can you please check to see if you have browsing history clearing on exit? You can see this on brave://settings/clearBrowserData under Basically, the behavior should be matching Chromium completely now (well, except that Brave is missing customize feature which Chromium has, captured with #7493). You can test which tiles Chrome would show by visiting brave://new-tab-page and you should see the same tiles there |
@bsclifton I have brave set to delete history when the app closes. And as for that new tab, I don't use google. Maybe make something like that except with duckduckgo or qwant? |
@MikeScotty you don't have to use Google - you can open brave://new-tab-page in Brave and just verify the same thing shows for you there (should be the same as brave://newtab) |
Is this problem likely to be fixed? |
@MikeScotty we just landed a new top sites experience in Nightly which you can check out - I'm curious if this works any better. It should now have the customize mode which may persist after history is cleared. If you're on release channel, it'll be working it's way down on April 13th |
Test plan
See brave/brave-core#6584
Description
The top sites disappear after closing and opening brave
Steps to Reproduce
Actual result:
Top sites are gone
Expected result:
Top sites remain where they #were
Reproduces how often:
Easily reproduced
Brave version (brave://version info)
1.13.82 Chromium: 85.0.4183.83 (Official Build) (64-bit)
Version/Channel Information:
Other Additional Information:
Miscellaneous Information:
I also have brave set to delete history after closing the app but that never caused the top sites to disappear before
The text was updated successfully, but these errors were encountered: