Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make history page remember last query string & search limit #5192

Open
wants to merge 9 commits into
base: development
Choose a base branch
from
Open

Make history page remember last query string & search limit #5192

wants to merge 9 commits into from

Conversation

PikachuEXE
Copy link
Collaborator

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

Description

Make history page remember last query string & search limit to deal with

  • Search in history
  • Found a few videos
  • View one of them, go back last page, oops not filtered

Screenshots

No video I am lazy

Testing

  • Ensure last used search query and data limit restored when revisiting history page
  • Ensure they are not restored after clearing query text / in new window
  • Ensure no 2 time rendering on page initial render

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

There are 2 commits & first one is refactor
Might easier to review per commit

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 30, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 30, 2024 06:34
@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented May 30, 2024 •
edited
Loading

I understand where you're are coming from but this behavior is also not apparent in Playlists tab, Channel tab, and a Channel page.

Channel page one also applies to Nr 6 on the list of issue #2446 which is a way bigger issue

Just to clarify im not against this but I think we should do it for all pages instead of one for the sake of a consistent UX

@kommunarr
Copy link
Collaborator

I'd also like to know rationale, as I don't see any requests for this. I'm not necessarily opposed to it, but I don't have any reason to believe that the app remembering my last query would cause fewer irksome moments than the app not remembering it.

@PikachuEXE
Copy link
Collaborator Author

Making it work like search query page
I realize this when searching in history (and several videos popup), click one of them, not the one I want, go back, oops
Though I can always open in new windows (but mobile can't do it?)
Up to you guys

@kommunarr
Copy link
Collaborator

kommunarr commented May 31, 2024 •
edited
Loading

If you want, you can share the build & I can try it out for a while in personal use. It sounds nice in theory, but it feels more horizontal of a change for the use cases where you want to go to the initial page state & now have to click to clear the input. The search bar is also omnipresent, so you wouldn't ever forget what was going on, but here I could see it leading to momentary confusion and re-familiarizing yourself with the context of what's going on & what you were searching beforehand. Seems like maybe "open in a new tab" covers the type of use case you're mentioning better (albeit I guess not if you didn't expect to misclick).

@PikachuEXE PikachuEXE force-pushed the feature/history/remember-search-query branch from 434c7d5 to cfbda54 Compare June 3, 2024 05:49
@PikachuEXE
Copy link
Collaborator Author

PikachuEXE commented Jun 3, 2024 •
edited
Loading

Used router.replace to ensure the query text restored only on clicking back
Clicking on history link won't restore it

About the catch after replace:
https://v3.router.vuejs.org/guide/advanced/navigation-failures.html

Update 1: Use my custom build if you want - https://github.com/PikachuEXE/FreeTube/actions/runs/9345433122

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

Just to clarify im not against this but I think we should do it for all pages instead of one for the sake of a consistent UX

Just retested it but my opinion on this hasnt changed.

@PikachuEXE
Copy link
Collaborator Author

@efb4f5ff-1298-471a-8973-3d47447115dc
Can you list all the page types so I won't miss any on next update?

@kommunarr
Copy link
Collaborator

That sounds like it would be a pretty sizable change to the codebase for a small feature. I would personally advise putting this one on the backburner just because I'm not sure if the feature would provide enough utility versus the effort to maintain it.

@github-actions GitHub Actions
Copy link
Contributor

This PR is stale because it has been open 28 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc Can you list all the page types so I won't miss any on next update?

Sure:

  • FT Channel tab
  • User Playlist tab main view
  • Inside User Playlist
  • Creator Channel page

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Aug 1, 2024 •
edited
Loading

Q: Why does it take a while for the thumbnails to appear when navigating back to the History tab? Why isnt it instantly shown?

VirtualBoxVM_bZgbgCWrV7.mp4

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Aug 16, 2024
@github-actions GitHub Actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@PikachuEXE PikachuEXE force-pushed the feature/history/remember-search-query branch from 3c71c71 to 987b10a Compare August 20, 2024 01:12
@PikachuEXE
Copy link
Collaborator Author

Q: Why does it take a while for the thumbnails to appear when navigating back to the History tab? Why isnt it instantly shown?

Fixed

@github-actions GitHub Actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@PikachuEXE
Copy link
Collaborator Author

FT Channel tab
User Playlist tab main view
Inside User Playlist
Creator Channel page

Also done but for the Creator Channel page one I am not really confident (even though I tested it a bit

@PikachuEXE PikachuEXE added the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 20, 2024
@PikachuEXE PikachuEXE force-pushed the feature/history/remember-search-query branch from a170f96 to 2f8fa91 Compare September 3, 2024 00:44
@PikachuEXE
Merge branch 'development' into feature/history/remember-search-query
47222b9 
* development: (74 commits)
  Translated using Weblate (Flemish (West))
  Added translation using Weblate (Flemish (West))
  Translated using Weblate (Slovak)
  Improve history import performance and fix some bugs (FreeTubeApp#5666)
  Bump electron from 32.0.2 to 32.1.0 (FreeTubeApp#5710)
  Local API: Use IOS HLS manifest for livestreams (FreeTubeApp#5705)
  Translated using Weblate (Slovak)
  Translated using Weblate (Japanese)
  Bump the stylelint group with 2 updates (FreeTubeApp#5706)
  Bump swiper from 11.1.12 to 11.1.14 (FreeTubeApp#5709)
  Fix a few memory leaks while tearing down the player (FreeTubeApp#5698)
  Fix audio track selection (FreeTubeApp#5697)
  Bump shaka-player from 4.10.12 to 4.11.1 (FreeTubeApp#5677)
  Translated using Weblate (Czech)
  Translated using Weblate (Serbian)
  Translated using Weblate (Polish)
  Translated using Weblate (German)
  Translated using Weblate (Chinese (Simplified Han script))
  Bump express from 4.19.2 to 4.20.0 (FreeTubeApp#5687)
  Translated using Weblate (Turkish)
  ...
@github-actions GitHub Actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 20, 2024
@PikachuEXE
Merge branch 'development' into feature/history/remember-search-query
ce7c712 
* development:
  Fix video start time handling (FreeTubeApp#5719)
  Local API: Handle new shorts node (FreeTubeApp#5679)
  Update subscription cache when subscribing from the channel page (FreeTubeApp#5717)
  Translated using Weblate (English (United Kingdom))
  Implement persistent subscription cache (FreeTubeApp#5185)
  Translated using Weblate (Belarusian)
  Bump npm-run-all2 from 6.2.2 to 6.2.3 (FreeTubeApp#5708)
  Translated using Weblate (Swedish)
  Bump shaka-player from 4.11.1 to 4.11.3 (FreeTubeApp#5707)
  Translated using Weblate (Persian)
  v Downgrade electron-builder 25.x > 24.x (FreeTubeApp#5712)
  Translated using Weblate (Persian)
  Translated using Weblate (Persian)

# Conflicts:
#	src/renderer/views/Channel/Channel.js
#	src/renderer/views/Channel/Channel.vue
@github-actions GitHub Actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@PikachuEXE PikachuEXE added the PR: waiting for review For PRs that are complete, tested, and ready for review label Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc Awaiting requested review from efb4f5ff-1298-471a-8973-3d47447115dc

At least 3 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
PR: waiting for review For PRs that are complete, tested, and ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc @kommunarr