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

Fix library filter state not being saved on client #4697

Closed

Conversation

ricci2511
Copy link

Changes
Added saveQuery function calls on several filterchange event callbacks to properly save the filter state on the client.

Issues
Fixes #40

@ricci2511 ricci2511 requested a review from a team as a code owner June 22, 2023 00:21
@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

No Coverage information No Coverage information
26.7% 26.7% Duplication

@ricci2511
Copy link
Author

ricci2511 commented Jun 22, 2023

Maybe to avoid duplication it would be worth it to abstract the this.showFilterMenu function callback to its own function, e.g. like this:

function showFilterMenuCb(query, mode, filterchangeCb) {
    import('../../components/filterdialog/filterdialog').then(({ default: filterDialogFactory }) => {
        const filterDialog = new filterDialogFactory({
            query,
            mode,
            serverId: ApiClient.serverId()
        });
        Events.on(filterDialog, 'filterchange', filterchangeCb);
        filterDialog.show();
    });
}

And then e.g. in movies.js we could replace the current this.showFilterMenu implementation with this:

    this.showFilterMenu = showFilterMenuCb.bind(this, query, 'movies', () => {
        query.StartIndex = 0;
        userSettings.saveQuerySettings(savedQueryKey, query);
        itemsContainer.refreshItems();
    });

@thornbill thornbill added the enhancement Improve existing functionality or small fixes label Jul 6, 2023
@thornbill
Copy link
Member

I have a concern with this... our current UI does not have any indication when a filter is active. I can see this causing an influx of issues where people have forgotten they set a filter and are confused why some of their content is not being displayed.

@ricci2511
Copy link
Author

Understandable, I guess I could look into implementing such indicator if desired.

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Sep 8, 2023
@jellyfin-bot
Copy link
Collaborator

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@thornbill
Copy link
Member

This was implemented in #5717

@thornbill thornbill closed this Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality or small fixes merge conflict Conflicts prevent merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Library Filter Does Not Save At All
3 participants