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

fix: duplicated file search request #9861

Merged
merged 2 commits into from
Oct 26, 2023
Merged

fix: duplicated file search request #9861

merged 2 commits into from
Oct 26, 2023

Conversation

fschade
Copy link
Contributor

@fschade fschade commented Oct 25, 2023

Description

fixes a bug where the search was sent unnecessarily twice.

Related Issue

How Has This Been Tested?

  • manual, unit and e2e

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@fschade fschade self-assigned this Oct 25, 2023
@fschade fschade added the Status:Needs-Review Needs review from a maintainer label Oct 25, 2023
AlexAndBear
AlexAndBear requested changes Oct 25, 2023
View reviewed changes
packages/web-app-files/src/components/Search/List.vue Outdated
emit('search', buildSearchTerm(true))
// return early if this view is not active, no search needed
{
const isSearchViewPainted = isLocationCommonActive(router, 'files-common-search')
Copy link
Contributor

Choose a reason for hiding this comment

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

Painted ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, painted, e.g. painted on screen, rendered, visible, ...

Copy link
Contributor Author

@fschade fschade Oct 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

you like active better? but keep in mind, active can mean, active in background or similar and on screen

Copy link
Contributor

Choose a reason for hiding this comment

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

"Mounted", as per Vue-jargon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i updated it to isSearchViewActive since the router guard is also called isLocationCommonActive, thanks guys for the input

@fschade fschade force-pushed the fix-duplicated-search-request branch from 8620af5 to 4ce4a07 Compare October 25, 2023 10:59
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@JammingBen JammingBen merged commit ce03415 into master Oct 26, 2023
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the fix-duplicated-search-request branch October 26, 2023 07:37
@fschade fschade mentioned this pull request Oct 29, 2023
Merged
9 tasks
@micbar micbar mentioned this pull request Dec 12, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JammingBen JammingBen JammingBen left review comments

@AlexAndBear AlexAndBear AlexAndBear approved these changes

Assignees

@fschade fschade

Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Duplicated search request on enter
3 participants
@fschade @AlexAndBear @JammingBen