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

Update ft-input for top navbar search input to behave more like Youtube one #3793

Merged
merged 7 commits into from Aug 2, 2023
Merged

Update ft-input for top navbar search input to behave more like Youtube one #3793

merged 7 commits into from Aug 2, 2023

Conversation

PikachuEXE
Copy link
Collaborator

@PikachuEXE PikachuEXE commented Jul 19, 2023 •
edited

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

Fixes #3068

Description

Update ft-input for top navbar search input to behave more like Youtube one

Fixes "searchbar isnt grabbing the words in the suggestion list"

Screenshots

Screen.Recording.2023-07-19.at.14.58.11.mov

Testing

A. Search bar input

B. Other inputs

  • No behaviour change should be observed (not sure about the specific stuff to be tested)

Desktop

  • OS:
  • OS Version:
  • FreeTube version:

Additional context

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) July 19, 2023 07:00
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jul 19, 2023
@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

In the video under the Expected Behavior header in #3068 around the 8sec mark u will see that when i press space bar while hovering on a search suggestion it creates a new list of search suggestions based on the search suggestion i was hovering on. That also needs to be implemented here.

efb4f5ff-1298-471a-8973-3d47447115dc
efb4f5ff-1298-471a-8973-3d47447115dc requested changes Jul 19, 2023
View reviewed changes
Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving with the left or right key

Current behavior:

Removes search suggestion

VirtualBoxVM_sImYBqMdsm.mp4

Expected behavior:

Doesnt remove search suggestion

firefox_YTy235aGO7.mp4

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Jul 19, 2023
@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

Clicking on searchbar will show previous hovered on search suggestion in the searchbar:

Current behavior:

Is showing previous hovered on search suggestion when u click on it

VirtualBoxVM_CCVq1mRi5W.mp4

Expected behavior:

Previous hovered search sugestion remains in the searchbar. Important note when u click on the searchbar u will see the search results of pikachu and not pikachu drawing

firefox_qOnkxqXG6e.mp4

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

Clearing searchbar doenst work on a search suggestion?

VirtualBoxVM_lu6fyd8LhB.mp4

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

Switched back a few versions because something else looked off to me in this PR.

Current behavior:
Search suggestions will be displayed in the search bar in 2 ways

  1. Using the up and down arrow keys on your keyboard
  2. Hovering on the search suggestion
VirtualBoxVM_SYv9Ew3QVM.mp4

Expected behavior:
Hovering over the search suggestion shouldn't impact anything

firefox_DxVVMyZBNa.mp4

@PikachuEXE
Copy link
Collaborator Author

In short the expected behaviour is whatever YT is doing...

@PikachuEXE PikachuEXE changed the title Update ft-input for top navbar search input to display selected search suggestion value in input box Update ft-input for top navbar search input to behave more like Youtube one Jul 20, 2023
@PikachuEXE
Copy link
Collaborator Author

"mouseleave on search suggestion = deselect suggestion" behaviour intentionally not implemented
If you think that behaviour should be copied too I can add it

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

In short the expected behaviour is whatever YT is doing...

Yeah, we were doing all this in the past but idk what happened. Thats why i requested so many changes.

"mouseleave on search suggestion = deselect suggestion" behaviour intentionally not implemented If you think that behaviour should be copied too I can add it

Yes please, i think i also requested this but cant find it because i requested allot or maybe i wanted to request it. idk anymore..

efb4f5ff-1298-471a-8973-3d47447115dc
efb4f5ff-1298-471a-8973-3d47447115dc requested changes Jul 21, 2023
View reviewed changes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. When i press spacebar on a search suggestion it should generate new search suggestions based on the one i pressed spacebar one
VirtualBoxVM_MCZK7QKbXJ.mp4
firefox_tRBGLW7JGK.mp4
  1. Moving with the left or right key

Current behavior:

Removes search suggestion

VirtualBoxVM_sImYBqMdsm.mp4

Expected behavior:

Doesnt remove search suggestion

firefox_YTy235aGO7.mp4
  1. clearing searchbar when searchsuggestion replaced original keyword doenst work
VirtualBoxVM_RVd2lztIri.mp4

@PikachuEXE
Copy link
Collaborator Author

I think I fixed all 3
Please check

Maybe retest previous stuff too :P

efb4f5ff-1298-471a-8973-3d47447115dc
efb4f5ff-1298-471a-8973-3d47447115dc previously approved these changes Jul 25, 2023
View reviewed changes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is working flawlessly, LGTM!

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Jul 25, 2023
absidue
absidue approved these changes Jul 29, 2023
View reviewed changes
src/renderer/helpers/strings.js Outdated Show resolved Hide resolved
src/renderer/views/Channel/Channel.js Outdated Show resolved Hide resolved
absidue
absidue requested changes Jul 29, 2023
View reviewed changes
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't mean to approve

absidue
absidue reviewed Aug 1, 2023
View reviewed changes
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it and couldn't find any problems. Please fix that unused import linter warning please though.

src/renderer/helpers/utils.js Outdated
Comment on lines 7 to 9

import { isNullOrEmpty } from './strings'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the linter warning about this import being unused in this file is correct.

Suggested change
import { isNullOrEmpty } from './strings'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WIP change that I forgot to remove before committing
Removed now

@PikachuEXE
Copy link
Collaborator Author

@ChunkyProgrammer Chunky power needed
image

@FreeTubeBot FreeTubeBot merged commit d8ed6b9 into FreeTubeApp:development Aug 2, 2023
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Aug 2, 2023
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Aug 5, 2023
@PikachuEXE
Merge branch 'development' into feature/playlist-2023-05
68ab480 
* development: (33 commits)
  Miscellaneous CSS cleanup (FreeTubeApp#3847)
  Fix empty channels showing up as errored with RSS (FreeTubeApp#3824)
  Fix author for album playlists on the playlist page (FreeTubeApp#3838)
  Update Snap Source Host Location (FreeTubeApp#3844)
  * Show error message in popular tab when instance does not support it (FreeTubeApp#3841)
  Use video durations from the watch history for RSS (FreeTubeApp#3839)
  ! Fix unnecessary error message display in toast when paused before video started playing on load (FreeTubeApp#3835)
  Use emit and props instead of $parent (FreeTubeApp#3834)
  Add custom toast event bus for Vue 3 compatiblity (FreeTubeApp#3833)
  Fix handling of DeArrow titles (FreeTubeApp#3825)
  * Update top nav bar icon to remove focus state style (FreeTubeApp#3792)
  Update ft-input for top navbar search input to behave more like Youtube one (FreeTubeApp#3793)
  Translated using Weblate (Hungarian)
  Fix: importing subscriptions with terminated channels (FreeTubeApp#3816)
  Fix outdated subscription cache clearing code when "Remove All Subscriptions / Profiles" performed (FreeTubeApp#3817)
  Translated using Weblate (Croatian)
  Bump eslint-plugin-import from 2.27.5 to 2.28.0 (FreeTubeApp#3827)
  Bump eslint from 8.45.0 to 8.46.0 (FreeTubeApp#3829)
  Bump eslint-plugin-unicorn from 48.0.0 to 48.0.1 (FreeTubeApp#3828)
  Bump lefthook from 1.4.6 to 1.4.7 (FreeTubeApp#3830)
  ...

# Conflicts:
#	src/renderer/components/ft-list-video/ft-list-video.js
#	src/renderer/components/playlist-info/playlist-info.js
#	src/renderer/components/playlist-info/playlist-info.vue
#	src/renderer/components/watch-video-info/watch-video-info.js
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Aug 5, 2023
@PikachuEXE
Merge branch 'feature/playlist-2023-05' into custom-builds/current
734a56e 
* feature/playlist-2023-05: (31 commits)
  Miscellaneous CSS cleanup (FreeTubeApp#3847)
  Fix empty channels showing up as errored with RSS (FreeTubeApp#3824)
  Fix author for album playlists on the playlist page (FreeTubeApp#3838)
  Update Snap Source Host Location (FreeTubeApp#3844)
  * Show error message in popular tab when instance does not support it (FreeTubeApp#3841)
  Use video durations from the watch history for RSS (FreeTubeApp#3839)
  ! Fix unnecessary error message display in toast when paused before video started playing on load (FreeTubeApp#3835)
  Use emit and props instead of $parent (FreeTubeApp#3834)
  Add custom toast event bus for Vue 3 compatiblity (FreeTubeApp#3833)
  Fix handling of DeArrow titles (FreeTubeApp#3825)
  * Update top nav bar icon to remove focus state style (FreeTubeApp#3792)
  Update ft-input for top navbar search input to behave more like Youtube one (FreeTubeApp#3793)
  Translated using Weblate (Hungarian)
  Fix: importing subscriptions with terminated channels (FreeTubeApp#3816)
  Fix outdated subscription cache clearing code when "Remove All Subscriptions / Profiles" performed (FreeTubeApp#3817)
  Translated using Weblate (Croatian)
  Bump eslint-plugin-import from 2.27.5 to 2.28.0 (FreeTubeApp#3827)
  Bump eslint from 8.45.0 to 8.46.0 (FreeTubeApp#3829)
  Bump eslint-plugin-unicorn from 48.0.0 to 48.0.1 (FreeTubeApp#3828)
  Bump lefthook from 1.4.6 to 1.4.7 (FreeTubeApp#3830)
  ...
@PikachuEXE PikachuEXE deleted the fix/search-bar-suggesion branch August 23, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@absidue absidue absidue approved these changes

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc approved these changes

@ChunkyProgrammer ChunkyProgrammer ChunkyProgrammer approved these changes

@MarmadileManteater MarmadileManteater Awaiting requested review from MarmadileManteater

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: searchbar isnt grabbing the words in the suggestion list
5 participants
@PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc @absidue @ChunkyProgrammer @FreeTubeBot