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

Bookmarks library pane displays everything from the root down #1951

Closed
grigoryk opened this issue Apr 23, 2019 · 14 comments
Closed

Bookmarks library pane displays everything from the root down #1951

grigoryk opened this issue Apr 23, 2019 · 14 comments
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. eng:qa:verified QA Verified eng:ready Ready for engineering Feature:Bookmarks P1 Current sprint

Comments

@grigoryk
Copy link
Contributor

grigoryk commented Apr 23, 2019

This doesn't seem correct. In Fennec we pretend our root is the mobile folder. toolbar folder is meaningless in Fenix, as well. It's nice to be able to manage it though!

One consideration we've had in the past is data integrity due to buggy bookmark syncing, but perhaps with Rust-powered dogear bookmark merging, those could be put to rest?

cc @linacambridge

ScreenshotUNITO-UNDERSCORE!1556054954!

┆Issue is synchronized with this Jira Task

@grigoryk grigoryk added 🐞 bug Crashes, Something isn't working, .. Feature:Bookmarks labels Apr 23, 2019
@vesta0 vesta0 added the needs:UX-feedback Needs UX Feedback label Apr 24, 2019
@vesta0 vesta0 added this to the Bugs milestone Apr 24, 2019
@vesta0 vesta0 added the P1 Current sprint label Apr 24, 2019
@topotropic
Copy link

We don't sync bookmarks yet so we should hide the "menu", "toolbar", "unfiled" folders and launch people directly in "mobile" when they open bookmarks

@sblatz
Copy link
Contributor

sblatz commented Apr 26, 2019

I posted my thoughts on this here.

colintheshots added a commit to colintheshots/fenix that referenced this issue Apr 29, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Apr 29, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Apr 29, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Apr 29, 2019
@grigoryk
Copy link
Contributor Author

grigoryk commented Apr 29, 2019

To circle back here and document what we've discussed in Slack:

Let's do what we do in Fennec and on iOS. By default, in the Bookmarks pane we'll display contents of the mobile root, with a special folder pinned to the top named "Desktop Bookmarks", which contains desktop roots (but, with friendlier names - e.g. Toolbar, not toolbar).

This is similar to what @sblatz proposed in #2087, but with some more nuance. Since we have bookmark management, whatever we display should reflect actual structure of the data. For that reason, we can't "merge" all desktop bookmarks; moves from "unsorted" into "toolbar" will be confusing, since it'll look like nothing moved at all!

It's also quite valuable to be able to see contents of these folders if you're a bookmark user. It mirrors what we display in Desktop, and lets you browse your bookmarks in a familiar way as well as fully manage them if you need to.

@linabutler
Copy link
Member

I like @grigoryk's idea, too—show mobile as the default view, and squirrel away toolbar, menu, and unfiled under a fake "Desktop Bookmarks" folder. Here are the localizable names that Desktop uses for those roots.

@vladikoff
Copy link
Contributor

It's also quite valuable to be able to see contents of these folders if you're a bookmark user. It mirrors what we display in Desktop, and lets you browse your bookmarks in a familiar way as well as fully manage them if you need to.

👍

I like @grigoryk's idea, too—show mobile as the default view, and squirrel away toolbar, menu, and unfiled under a fake "Desktop Bookmarks" folder. Here are the localizable names that Desktop uses for those roots.

👍

This sounds good. As of f1050ea if the user starts with Desktop and syncs some bookmarks, they would get "no bookmarks" and no mobile folder.

@colintheshots colintheshots added the eng:qa:needed QA Needed label May 1, 2019
@project-bot project-bot bot moved this from In Progress to Ready for QA in Fenix Sprint Kanban May 1, 2019
@sv-ohorvath
Copy link
Contributor

sv-ohorvath commented May 2, 2019

@colintheshots Not sure I understand correctly, please correct me if I'm wrong:

  1. I should get a Desktop bookmarks folder if I sync with desktop
  2. No desktop or other folders on my device unless I have some mobile bookmarks as well and sync.

note: I don't get any synced desktop bookmarks or folders at the moment (master build 5/2). I do see websites as bookmarked which are synced from desktop, and if I edit them I see they are in the toolbar/menu folders.

@lime124 lime124 added the eng:ready Ready for engineering label May 2, 2019
@colintheshots
Copy link
Contributor

colintheshots commented May 5, 2019

@sv-ohorvath So we're partway to a solution. #1951 (comment) suggested we should start from the mobile root. So we're doing that now. However, bookmarks sync just landed so people want to see their desktop bookmarks on mobile.

The issue is there's no UX I've seen showing how the roots should be structured. I'm guessing I should basically copy Fennec and add a Show Desktop button, but I'd like UX feedback on this option. I assume I can just add a button to the mobile root that lets the user reach the desktop roots.

This particular ticket is around how we used to show the top-level root with all of the roots in it that the user couldn't actually edit. It wasn't a great experience and it's now gone.

@colintheshots colintheshots added the needs:UX-feedback Needs UX Feedback label May 5, 2019
@sv-ohorvath
Copy link
Contributor

@colintheshots Ok, thanks! From the comments, I thought there's something more here.
Wouldn't it be better to close this then and have a new ticket for UX to determine how we want to show the desktop bookmarks? There is one already reported: #2252.

@sv-ohorvath sv-ohorvath removed the eng:qa:needed QA Needed label May 6, 2019
@sv-ohorvath sv-ohorvath added the eng:qa:verified QA Verified label May 6, 2019
@bsurd bsurd moved this from Ready for QA to Done in Fenix Sprint Kanban May 6, 2019
@topotropic
Copy link

I'm guessing I should basically copy Fennec and add a Show Desktop button, .... I assume I can just add a button to the mobile root that lets the user reach the desktop roots.

Sounds good to me. Thanks for requesting UX feedback!

@topotropic topotropic removed the needs:UX-feedback Needs UX Feedback label May 6, 2019
@grigoryk
Copy link
Contributor Author

grigoryk commented May 6, 2019

@colintheshots @topotropic please see suggested solution above - #1951 (comment) - that should be a good starting point for what we should do here.

Bulk of the discussion on this topic is in this issue, so it makes sense to me to keep it that way (vs opening new issues).

@bifleming
Copy link

@mheubusch @vesta0 It appears that the original issue has been resolved, but we need a new issue for possible UX changes. Can one of you confirm and create the new issue if needed for MVP?

@vesta0
Copy link
Collaborator

vesta0 commented May 6, 2019

@bifleming we can close this issue but we need to address the issue reported here #2252 with the direction to add a Show Desktop button.

@bifleming
Copy link

@colintheshots pls see Vesta's comment above.

@bifleming bifleming removed this from Done in Fenix Sprint Kanban May 7, 2019
@vesta0 vesta0 added this to High priority backlog (not in the current sprint) in Fenix Sprint Kanban May 8, 2019
@bifleming bifleming moved this from High priority backlog (not in the current sprint) to UX Review in Fenix Sprint Kanban May 8, 2019
@bifleming bifleming moved this from UX Review to Done in Fenix Sprint Kanban May 8, 2019
@ghost
Copy link

ghost commented May 22, 2019

This is tracked in #2252

@data-sync-user data-sync-user changed the title Bookmarks library pane displays everything from the root down FNX2-17528 ⁃ Bookmarks library pane displays everything from the root down Aug 4, 2020
@data-sync-user data-sync-user changed the title FNX2-17528 ⁃ Bookmarks library pane displays everything from the root down FNX3-15933 ⁃ Bookmarks library pane displays everything from the root down Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX3-15933 ⁃ Bookmarks library pane displays everything from the root down FNX-5348 ⁃ Bookmarks library pane displays everything from the root down Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX-5348 ⁃ Bookmarks library pane displays everything from the root down Bookmarks library pane displays everything from the root down May 19, 2022
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 eng:ready Ready for engineering Feature:Bookmarks P1 Current sprint
Projects
None yet
Development

No branches or pull requests