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 URL update not reflected in list hook #350

Merged
merged 2 commits into from
Feb 8, 2020

Conversation

WithoutPants
Copy link
Collaborator

Fixes #348

Refactored ListHook again so that it initialises from interface forage once before the rest of the component is even rendered. When the forage is loaded, it initialises the filter from the saved value if and only if the search parameters on the URL are not present. Once this has been done, any updates to the current URL are applied to the filter.

Tests performed:

  • navigating to other screens then back to the scenes page maintains the same query filter
  • clicking on the tag/performer link correctly updates the filter and results
  • navigating to other pages in the results works as expected

@WithoutPants WithoutPants added the bug Something isn't working label Feb 5, 2020
@Leopere Leopere mentioned this pull request Feb 5, 2020
10 tasks
@bnkai
Copy link
Collaborator

bnkai commented Feb 5, 2020

Above tests work ok for me also.
Setting different filters for scenes/performers/studios , closing browser reopening works fine also

A minor glitch for test 2
When clicking on the performer link (small thumbnail/tag) the filter is updated but the results are not with the current sorting order but with the default instead
Before clicking the thumbnail
http://localhost:9998/scenes?disp=0&p=1&perPage=40&sortby=created_at&sortdir=desc
After (edited and cleaned)
http://localhost:9998/scenes?c={ type : performers , value :[{ id : 66 , label : Performer Name }], modifier : INCLUDES_ALL }&disp=0&p=1&perPage=40&sortby=date&sortdir=asc
I would expect current sortby and sortdir to be kept (created_at and desc)

edit
the same issue extends to perPage setting

@WithoutPants
Copy link
Collaborator Author

I was going to leave that issue for a later date, but I came up with a reasonable solution that I hope doesn't cause any other regressions. The solution was to change the links to exclude items per page, sort by, sort direction and display mode. Then when the list hook is initialised, if the URL is set and those items are not present in the URL, then they are initialised from the local forage instead.

The only thing I have tested with this latest commit is the paging, since I don't have sufficient data on the machine I'm on.

@bnkai
Copy link
Collaborator

bnkai commented Feb 6, 2020

Seems to work ok to me.

@WithoutPants WithoutPants merged commit c99ba68 into stashapp:develop Feb 8, 2020
@WithoutPants WithoutPants deleted the fix_scene_anchors branch May 15, 2020 07:13
Tweeticoats pushed a commit to Tweeticoats/stash that referenced this pull request Feb 1, 2021
* Fix URL update not reflected in list hook

* Maintain query prefs on tag click
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] Performer/Tag links in scenes page do not work
2 participants