Page bookmarks
Page bookmarks
Pull Request Type
- [x] Feature Implementation
Related issue
closes #1505 closes #4040 closes #312 partial alternative for #4594 provides setup for #1492
Description
- The ability to bookmark routes (not just keyword searches) through a button on the top nav
- Be able to set a named alias for those bookmarks through a modal popup
- When the current route is saved, it shows as starred in the top nav
- Clicking on the top nav for a bookmarked route opens a modal allowing you to edit the name and/or remove the bookmark
- Searching for a substring of a bookmark link or alias in the search bar shows the corresponding bookmarks at the top of the search results, with visual & ARIA identifiers
- Navigating to a page bookmark clears the input
- Default route name is chosen based off of the
document.title, playlist name, or search query depending on route - Delete all bookmarks setting in Privacy Settings
- Thanks to the full path of routes being stored, including parameters, "Search" bookmarks are restored with the same search filter settings they were made with
- If the current route is bookmarked, that bookmark does not show in the search results while still on that route
- The user is warned with an inline message, but not barred from doing so, when their current page bookmark name open in the modal is a duplicate of an existing page bookmark name
- The search results for a bookmarked result show an icon corresponding to the route type
- The search results are now limited to 15 max results, prioritizing page bookmarks first
- Search results are now each visually constrained to one line/row, due to much longer results being possible now
- Removing a page bookmarked user playlist also removes the page bookmark automatically (note: only applies to the main Playlist page(s) being page bookmarked)
Screenshots
Testing
- Test that bookmarks & bookmark removals are remembered after reloading
- Test that bookmark name updates are remembered and reflected after reloading
- Test that bookmarks are set to reasonable defaults for all types of page routes
- Test bookmark deletion in Privacy Settings
- Test that bookmarks appear when matching at the top of the search results
- Test that navigating bookmark search results with the Up/Down, Right/Left, and Enter keys behaves as expected
- Test that bookmarks appear and function properly even when
General Settings > Enable Search Settingsis disabled - Test that bookmark icon enabled/disabled states are discernible with
Theme Settings > Match Top Bar with Main Colorenabled (secondary color theme is used as fill) - Test that page bookmarking a user playlist page and deleting that playlist also removes the page bookmark
- Test that page bookmarking multiple user playlists' pages and then removing all playlists in Privacy Settings removes all of the page bookmarks for the now-deleted user playlist pages
- Test that page bookmark with a duplicate name shows a warning
Desktop
- OS: OpenSUSE
- OS Version: TW
Additional context
Future PRs
- Import/export bookmarks (somewhat related to #1426)
- (Needs refinement / discussion) Hub for viewing and editing all bookmarks
- (Questionable / needs discussion) Truncate search results such that they never wrap to a new line (this is apparent if you want to create exceptionally long playlist names as an edge case, but this wrapping of long search results onto new lines is already existing & encounterable behavior)
- (Questionable / needs discussion) Hide page bookmarks setting (separate from Enable Search Suggestions toggle)
- (Even more questionable / needs discussion) Have the set Search filter parameters update when taking in a saved Search Results page with given search parameters (more confusing than not, in my opinion)
- Search history (#1492): We can easily fit generic search history entries into this paradigm by creating
search-history.dbentries withisBookmark: falseset. Edit: probably should just leverage the existing search cache as the data source & existing access methods, but we can reuse the UI patterns added in this PR. I can revert https://github.com/FreeTubeApp/FreeTube/pull/5003/commits/392fa75a871c949edbda42b57eab9e80fef191bb, the commit I added specifically to better accommodate the possibility of a generic permanent search history through thesearch-history.db, if desired. It depends on whether we want to support a permanent search history and/or one with time-based expiry rules going forward.
Aspects open for discussion
- I keep the
- FreeTubepart in much of the default bookmark names, even though it's a bit ugly, because it helps to further visually (& search-wise) demarcate bookmarks, and is thus a good default.- Related: I keep
spellcheckon for the bookmark name insertionft-inputbecause we do it elsewhere for most of our inputs, even though it's a bit ugly, especially whenFreeTubeis underlined in red for most users. This one is debatable, but if I change it for this one, I'm also inclined to change it for other instances as well (e.g., User Playlist name insertion, profile name insertion).
- Related: I keep
- I put the Bookmark icon next to the search bar and filter button because it helps make it visually clear that it's related to them.
- Main routes like
/aboutare bookmarkable, even though they're equally accessible through the side nav. This might seem unnecessary, but having controls on the top nav phase in and out of being active seems like a poor & confusing choice in comparison. Having it be omnipresent like all of our other top nav icons, and communicating that any route works for it, makes the new feature more apparent and easier to understand/use.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This feature differs quite a bit from what I imagine it to be. Lets take the following artistic masterpiece as a starting point (this applies to #1505 and #1492):
ONLY when the user clicks on an empty search bar (imagine the search bar being empty in the pic above):
- The user will see the bookmarked searches listed on top, marked with a colored star.
- The user also sees the recent searches they made right beneath the bookmarked searches and can be removed by hitting the X.
When the user starts to type into the search bar:
- Bookmarked and recent searches will not be listed.
- Only search suggestion will be listed.
- User can only bookmark from the search suggestions list and can be added to the bookmarked list (see picture below)
Debatable:
- Bookmarking searches with filters applied to it. I think the user should stay in control of their bookmarks just with search suggestions. Apply the filter before/after you hit search
I dont see the need for:
- Bookmarking routes
- The popup to give the bookmark an alias
ONLY when the user clicks on an empty search bar (imagine the search bar being empty in the pic above): The user also sees the recent searches they made right beneath the bookmarked searches and can be removed by hitting the X.
Addressing #1492 is out of scope for this PR, IMO, unless we want to make it even bigger.
When the user starts to type into the search bar: Bookmarked and recent searches will not be listed.
I disagree on this one. This sets a hard limit on the number of bookmarks, makes searching bookmarks harder, and is different from most browser (and in the case of YT for search history) behavior that people are most familiar with, where those are put at the top of the results.
User can only bookmark from the search suggestions list and can be added to the bookmarked list (see picture below)
I disagree with this one for the reason that it's a prohibitive UX. You don't often think to save a page before you actually click on it, which is going to lead to a good deal of frustration. You probably want it to be somewhere visually constant so that you can bookmark a page whenever you want to. See my discussion from the OP:
Main routes like /about are bookmarkable, even though they're equally accessible through the side nav. This might seem unnecessary, but having controls on the top nav phase in and out of being active seems like a poor & confusing choice in comparison. Having it be omnipresent like all of our other top nav icons, and communicating that any route works for it, makes the new feature more apparent and easier to understand/use.
I dont see the need for: Bookmarking routes
See answers to the above on why I disagree. In short, the cognitive model of browser bookmarking is one that users know about and are more effortlessly familiar with, but the model you're proposing is one that users will have to put in some work to learning and understanding the behaviors you've laid out, as it's a novel (at least to my knowledge) system and ruleset.
I dont see the need for: The popup to give the bookmark an alias
I can see the argument for this one. I would be open to having the modal close action for this one be to save rather than cancel (similar to Firefox), but I do worry that's not as intuitive given that differs from our other modals. Alternatively, we could have bookmarking not open the modal by default, but a toast pops up presenting you the option to edit it (otherwise, you can just click the icon again).
This pull request has conflicts, please resolve those before we can evaluate the pull request.
ONLY when the user clicks on an empty search bar (imagine the search bar being empty in the pic above): The user also sees the recent searches they made right beneath the bookmarked searches and can be removed by hitting the X.
Addressing #1492 is out of scope for this PR, IMO, unless we want to make it even bigger.
Included that one just to make the examples easier to understand
When the user starts to type into the search bar: Bookmarked and recent searches will not be listed.
I disagree on this one. This sets a hard limit on the number of bookmarks, makes searching bookmarks harder, and is different from most browser (and in the case of YT for search history) behavior that people are most familiar with, where those are put at the top of the results.
Ah i didnt know. How does YT search history work? Could you maybe expand some more on the how browsers tackles bookmarks and how we do a similar job of it (i havent used browser bookmarks for a long time)
User can only bookmark from the search suggestions list and can be added to the bookmarked list (see picture below)
I disagree with this one for the reason that it's a prohibitive UX. You don't often think to save a page before you actually click on it, which is going to lead to a good deal of frustration. You probably want it to be somewhere visually constant so that you can bookmark a page whenever you want to. See my discussion from the OP:
Main routes like /about are bookmarkable, even though they're equally accessible through the side nav. This might seem unnecessary, but having controls on the top nav phase in and out of being active seems like a poor & confusing choice in comparison. Having it be omnipresent like all of our other top nav icons, and communicating that any route works for it, makes the new feature more apparent and easier to understand/use.
Ah ok I think I understand
I dont see the need for: Bookmarking routes
See answers to the above on why I disagree. In short, the cognitive model of browser bookmarking is one that users know about and are more effortlessly familiar with, but the model you're proposing is one that users will have to put in some work to learning and understanding the behaviors you've laid out, as it's a novel (at least to my knowledge) system and ruleset.
👍 im fine with it but please provided some examples of the familiar behaviors like requested above.
I dont see the need for: The popup to give the bookmark an alias
I can see the argument for this one. I would be open to having the modal close action for this one be to save rather than cancel (similar to Firefox), but I do worry that's not as intuitive given that differs from our other modals. Alternatively, we could have bookmarking not open the modal by default, but a toast pops up presenting you the option to edit it (otherwise, you can just click the icon again).
After testing some more i withdraw this. I think its very valuable to search for something with a filter applied and bookmark it with an alias so you exactly know what it is
How does YT search history work?
Incidentally, I'm thinking about us moving the star icon to the left of the label, adding a magnifying glass by regular results, adding a loop icon for history results, and adding remove icon or text button on the right side for history results.
Could you maybe expand some more on the how browsers tackles bookmarks and how we do a similar job of it (i havent used browser bookmarks for a long time)
Some behaviors vary based off of the specific browser and what settings you have enabled, but at least in every major browser, bookmarks and search history are prioritized over regular results by default. See discussion here on Chromium's behavior for example. As to which takes precedence over which, I think it varies. In Firefox, judging from a few minutes of testing, bookmarks and search history are equal, with relevance, closeness of order, and time of search seeming to be the apparent factors of sorting in order of priority descending.
im fine with it but please provided some examples of the familiar behaviors like requested above.
This is grabbed from Firefox:
Conflicts have been resolved. A maintainer will review the pull request shortly.
ONLY when the user clicks on an empty search bar (imagine the search bar being empty in the pic above): The user will see the bookmarked searches listed on top, marked with a colored star. The user also sees the recent searches they made right beneath the bookmarked searches and can be removed by hitting the X.
I disagree on the first one. I think we should only do this for search results, because this matches browser / YT behavior better.
Here are the builds in case you're interested in testing it out for your regular use. I've admittedly been pretty eager to incorporate this into my personal use.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
There was feedback provided in the Matrix chat from one developer that this might constitute creep. I see two ways of thinking about it:
-
I think I can see that bookmarking arbitrary pages can be much less useful, so there's a very reasonable argument to be made for restricting this feature to specific routes (page searches, and arguably user playlist pages). This would probably require some refactor to the UX & have it located in the page content, rather than in the top nav.
- Pros:
- No support for "useless" use cases
- Aligns with solutioning system of: there is always one way to solve any problem, and that's all that's needed
- Cons:
- Although this solution seems less wasteful, I have no idea what the mental model is for "saving searches" that isn't "bookmarking." What it will be is not a thing that most anyone has come into contact with, and thus it's most likely going to be less intuitive to understand & use than the current system.
- We could theoretically keep the existing UX but solely just have it disappear / be disabled on most routes, but then you run into another mental model issue of "bookmark icon that is seemingly arbitrarily absent / disabled a lot of the time." Even if we could find a way to make it non-confusing, such a change to the existing behavior seems very likely to cause more frustration than any noticeable benefit.
- Pros:
-
On the other hand, FreeTube is basically a super browser for navigating the certain websites that we cover, and there isn't much risk for confusion or other negative externalities if we lean a bit further into those strong mental models. Having a good & accessible UX means that users don't have to try hard to learn how to use your app. And so oftentimes, if users want to save a given page to go back to, sometimes they will want to bookmark it. The same way that even though YouTube has user playlists, and the ability to subscribe to channels, and so on, people still bookmark certain user playlists, and certain channels, and so on, because it's a more comfortable & familiar way for them to engage with the app, that probably has subtler conveniences to the flow of it (e.g., it's all under one system, I have a naming schema I use that makes accessing pages lightning fast, and so on).
- Pros:
- Aligns with the solutioning system of: whatever is easiest to use, we're all for it, even if it means more than one way that a problem could be solved.
- Aligns with the conventional mental model of browser page bookmarks, making it much easier for users to solve many common problem types, both for new use cases and even existing ones.
- Cons:
- Provides theoretical support for "useless" use cases (e.g., bookmarking the Subscriptions page).
- One could make an argument that it creates an opposing system to "Quick Bookmarks," thus confusing users.
- I will say, I don't find this one very convincing. Saving videos through that system is more accurately termed "favoriting" and "adding to a playlist" than "bookmarking". They're located in very different places spatially & behaviorally, letting us very easily occupy the two spaces of YouTube's playlist/favoriting mental model in the side nav and the browser's page bookmarking mental model by the search bar in the top nav.
- Deviates from the herd of the other similar apps, where this isn't a thing.
- For similar reasons to those given in my answer above, I think this is an example of what makes FreeTube unique. People using web applications like Piped and Invidious are most definitely leveraging their browser functionalities of page bookmarks and so on in their personal use instead of solely leveraging the capabilities provided by those websites directly. The benefit of FreeTube is that we are a whole application designed around your viewing experience. We're your browser to search and bookmark whatever pages you want. We're your vault of playlists. We're your private viewing history, with no strict need to have strong trust or commitment to any given host.
- Pros:
And so, barring some major missing cons to the second option that I'm most definitely missing (as I am oft to do), I think the main issue isn't about what problems are caused by this PR in its current state at a user level, but rather the philosophical problem of how we want to let users solve problems. I lean on the side of "whatever is easiest to use, we're all for it, even if it means more than one way that a problem could be solved" because it is - when done right (and not in the vein of "infinite feature creep, keep adding things to keep the developer brain happy") - the best UX. I can see why the other often opposing philosophy of removing any possible chaff is so appealing from an optimization perspective (Z is possible through way X, so requesting us to build Y is just putting more effort onto developers), and as a heuristic (if the minimum problem needed to be solved is X, adding more could just be adding more complexity & effort without a commensurate return in utility yielded).
But in this particular case, I think this is more clearly in favor of the former solutioning method because any other mental model or more limited form of the feature is perhaps unintuitively asking more of our users than its current state. I'm certainly willing to hear other opinions on it, especially if anyone else wants to try out the build for their own personal use and get beyond just the theoretical side of the discussion. Especially from non-developer-y people, because I do know we devs tend to get in our heads & worlds separate from what's actually going on.
A few comments after trying it out briefly
- Lacking cancel/close button on Edit Bookmark modal (especially for mobile)
- Lacking "page type" in default bookmark name (if the bookmark entries can be shown with icons/whatever indicating the page types this might not be a big issue)
- Deleting local playlist should auto delete related bookmark entry
- Being able to create bookmark entries with duplicate name is unexpected for me
Lacking cancel/close button on Edit Bookmark modal (especially for mobile)
From the Web side, the idea is that the Save button closes if no changes, similar to how it works in browsers like Chrome and FF (except in Chrome, the button is labeled Done). This is maybe a bit different feeling than those because it's a modal and not a context menu (which I also will add to the Future PRs section now). In the meantime, would a label of Done better communicate that?
For mobile, I honestly didn't think about it needing something different, and I'm glad you pointed it out. Interestingly, it seems like most mobile web browsers I'm testing with don't actually show a modal by default at all. Firefox Mobile saves the bookmark with a toast in one click, then lets you either re-click the icon to remove, press the toast to Edit, or go to a Bookmark Settings to edit it after you find it visually in the list. What about mobile one-click save, toast on save that opens the Modal, and re-click on a bookmarked video to open the modal?
Lacking "page type" in default bookmark name (if the bookmark entries can be shown with icons/whatever indicating the page types this might not be a big issue)
This one is interesting. I'm guessing you're referring to the custom routes where we show the playlist name or video title and not the translateWindowTitle default title routes. Yeah, I'm on board with a visual indicator of the page corresponding to each route (+ maybe a star, or the icon is colored the star color) in lieu of the star icon when it appears in the search results.
Deleting local playlist should auto delete related bookmark entry
This one is interesting as well. I could see it being convenient for the use case of "get rid of this video everywhere", having them linked like this, but I'm not sure if that would ever be undesirable. Maybe I'll add this one with an undo toast.
Being able to create bookmark entries with duplicate name is unexpected for me
This is in line with how browsers act, so I don't think it would be surprising / a bad UX as a user necessarily. I'll revisit this one when we have the route-specific icon, because in such a case having multiple same-named bookmarks would still be understandable if they're different pages for the same channel (e.g., their UULF playlist, their channel page, etc.,) or different gradients of the same content (e.g., a "GOAT music" video, a "GOAT music" playlist).
I am fine with Save only if when there are changes (new bookmark/edit with changes)
But having Done constantly seems easier & consistent
From a Chromium base browser:
To have mobile one-click save the default title must be good enough and it doesn't seem to be right now (also consider usually web pages got more unique titles unlike inside FT) Also in mobile it's easy to mis-click, so I am not sure if one-click should be enabled in the first release version
I could see it being convenient for the use case of "get rid of this video everywhere"
I am talking about local playlist (not videos inside local playlist). Having a bookmark entry remained after local playlist deleted = can visit a page with deleted local playlist = unexpected for me No idea what "get rid of this video everywhere" means
This is in line with how browsers act
It's possible to have the same page/URL bookmarked in several folders (or even duplicate entries in the same folder) However browsers have a bookmark management page and we don't have I don't know how bad this can be after page type displayed but at least we should show a warning (with count maybe) when trying to create/edit an entry while entries with the same name exist
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Conflicts have been resolved. A maintainer will review the pull request shortly.
Playlists page & single playlist page (remote & local) bookmark entries got different icons?
Yes, see likely implementation for the inline user playlist icon for the motivation on that.
Could use better default titles, e.g. playlist to have channel name when exists like https://github.com/FreeTubeApp/FreeTube/pull/5226, same for videos
If we don't like the current titles we're using, could we modify the document.title for those routes accordingly in a separate PR instead of having one-off overrides throughout the codebase?
Right below that comment there is:
I wouldn't use fa-bars for the playlists button, as it is visually very similar to the icon used for the channels one.
https://github.com/FreeTubeApp/FreeTube/issues/4594#issuecomment-2115281762
I find it hard to distinguish one from another too
If we don't like the current titles we're using, could we modify the document.title for those routes accordingly in a separate PR instead of having one-off overrides throughout the codebase?
Let me do that later
Hm, any suggestions? What if we made the Channels tab user-check (separate PR, of course)?
Here's what that would look like:
(Side-note: regardless of this, we may need to create a PR normalizing icon width while preserving the original proportions, both here and in ft-select icons)
It might help but playlist (at least user playlist) should still use bookmark, no idea about remote playlist
@PikachuEXE How about this? It's a bit odd since we're using filled versus unfilled to signify Quick Bookmark icon versus non-Quick Bookmark, but that's in a separate enough context, so I think it should be understandable here.
Then the problem being not being able to remember solid/normal icons means local or remote playlist It's still better than before Still pending on me to get the title updated before re-testing (coz that might help