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.

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

Added a search button in the filter modal #6938

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

Conversation

MilkyLane
Copy link

@MilkyLane MilkyLane commented Feb 27, 2025

Title

This adds the feature "apply button to search filters modal"

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Related issue

#6194

Closes: #6194

Description

Added a new function that uses the same openInternnalPath function used by main search, a button, and a gap for the new button improvements. Tested functionality using console logs and trial-and-error. No known issues.

Screenshots

image

Testing

Pull request tested

Tested using console logs and trial-and-error to verify functionality.

No ramifications remaining

Desktop

  • OS: Windows
  • OS Version: 10, Version 22H2
  • FreeTube version: v0.23.2 Beta

Additional context

No additional context.

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Feb 27, 2025
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) February 27, 2025 13:21
@efb4f5ff-1298-471a-8973-3d47447115dc

see review comments on #6277 #6627 to improve your PR

@MilkyLane
Copy link
Author

see review comments on #6277 #6627 to improve your PR

I'm having trouble implementing the shift key functionality to open search results in a new window. The current implementation ive added detects the shift key press and tries to use ipcRenderer to send a 'createNewWindow' message (similar to how it's done in top-nav.vue) but it's not working. The normal search (without shift) works fine. Has anyone successfully implemented this behavior in other components? Any guidance would be much appreciated

@@ -248,7 +256,28 @@ watch(searchFilterValueChanged, (value) => {
function hideSearchFilters() {
store.dispatch('hideSearchFilters')
}
function searchWithFilters() {
const queryTextElement = document.querySelector('.searchInput input')
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, you shouldn't use document.querySelector. The appropriate way to do this would be to add a value to the utils store for the search query and pull it from there. Perhaps under searchSettings:

searchSettings: {
sortBy: 'relevance',
time: '',
type: 'all',
duration: '',
features: [],
},

auto-merge was automatically disabled March 12, 2025 10:36

Head branch was pushed to by a user without write access

@FreeTubeBot FreeTubeBot enabled auto-merge (squash) March 12, 2025 10:36
@MilkyLane
Copy link
Author

Updated PR has requested changes however i still could not get shift+click to open a new tab in the search filters view

@@ -56,7 +56,7 @@
text-color="var(--text-with-main-color)"
@click="hideSearchFilters"
/>
<!-- Added: Search button with same styling as close button -->

<FtButton
:label="$t('Search')"

Choose a reason for hiding this comment

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

localize the label

@@ -79,9 +79,18 @@ import FtButton from '../ft-button/ft-button.vue'
import FtCheckboxList from '../FtCheckboxList/FtCheckboxList.vue'

import store from '../../store/index'
import { useRouter } from 'vue-router'

Choose a reason for hiding this comment

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

why import it but never use it?

@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 Mar 12, 2025
@MarmadileManteater
Copy link
Contributor

Updated PR has requested changes however i still could not get shift+click to open a new tab in the search filters view

The event parameter on the searchWithFilters function is being passed through as undefined.

@MarmadileManteater
Copy link
Contributor

The reason it works on the ft-input is because of this line:

this.$emit('click', this.inputData, { event: e })

where the event is being passed through,

but on the ft-button

The event is not passed through.

Additionally, you should use the doCreateWindow option on openInternalPath instead of handling it yourself. It already checks for electron.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Add apply button to search filters modal
3 participants