-
Notifications
You must be signed in to change notification settings - Fork 287
[fix] Import applied language filter #4376
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
Conversation
|
@vkWeb could you take a look? |
|
@FidalMathew Assigned myself to this PR, will look soon. |
vkWeb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On selecting a language and removing it, I'm getting the below error:
It's happening because we are navigating user to the same URL twice.
Instead of using router.push, let's take a different approach here -- whenever languageFilter is changed in ChannelList, emit an event from there, you can do that in the computed property. Then we will catch that event on SearchOrBrowseWindow's ChannelList and assign a data property that will be sent as a query param to handleSearchTerm method.
|
@FidalMathew you nearly solved the issue, good job! its just that the error I mentioned above has popped up. Let's refactor 🚀 |
|
@vkWeb thank you! I'll work on it. |
|
@FidalMathew I'm looking forward to it! |
|
@vkWeb I have made the updated PR. Could you take a look at it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bug -- now, your changes are not allowing to set two languages together in import search.
I have requested changes.
contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ChannelList.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ChannelList.vue
Outdated
Show resolved
Hide resolved
...ation/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue
Outdated
Show resolved
Hide resolved
...ation/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue
Outdated
Show resolved
Hide resolved
vkWeb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FidalMathew I pushed one commit that changes some variable names, and did minor logic correction. Merging in. Thanks buddy, good work! :D
|
Thank you @vkWeb for guiding me through the issue! 😄 |
|
Thanks @FidalMathew - I've tested the issue at hotfixes and confirm that the language filter is displayed correctly now. |

Summary
Description of the change(s) you made
When the new search page opens, the filter is not applied, to tackle this issue, we are using the 'languages' param which is then imported by the language filter.
Manual verification steps performed
Screenshots (if applicable)
Before:
#4089 (comment)
After:

Does this introduce any tech-debt items?
At present, upon clicking 'back to browse,' the language selection is not retained. It resets to nothing. This decision was made due to the introduction of a new page where users can select multiple languages. Fetching the previously selected language(s) might lead to errors due to this change in the user flow.
Reviewer guidance
How can a reviewer test these changes?
References
Addresses #4089
Comments
Contributor's Checklist
PR process:
CHANGELOGlabel been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocslabel has been added if this introduces a change that needs to be updated in the user docs?requirements.txtfiles also included in this PRStudio-specifc:
notranslateclass been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages,components, andlayoutsdirectories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarnandpip)