Skip to content

Fix: persist listing sort order between page navigations #332

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

Merged
merged 6 commits into from
Jun 30, 2025

Conversation

allending313
Copy link
Contributor

This PR aims to solve this bug #196 where float/fade sort direction is not persisted between moving through listing result pages. This forces the user to manually re-apply the sort on every single page, which can be annoying.

Changes:

The fix uses a MutationObserver within SortListings to detect when the page content updates (user navigating to another page) and leverages a debounced handler to re-apply the user's chosen sort order.

A MutationObserver is initialized to watch the #searchResultsRows container for changes. The component doesn't re-render on page changes, so the MutationObserver is used to reliably detect when new items are loaded into the DOM, allowing us to trigger the re-sort logic.

The onMutation handler is debounced to ensure the function runs only once after all DOM changes have settled.

To prevent potential infinite loops, the onMutation handler disconnects the observer before modifying the DOM and reconnects it after the sorting is complete.

Screen.Recording.2025-06-11.at.5.36.06.PM.mov

Testing:

No build/run errors
Linter is happy
Screenshot 2025-06-11 at 5 12 48 PM

@Step7750 Step7750 requested review from GODrums, Step7750 and Copilot June 11, 2025 22:20
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a MutationObserver and debounced handler to the SortListings component so that the user’s chosen sort order is automatically re-applied when navigating between pages of listings.

  • Introduces connectedCallback/disconnectedCallback to watch the listing container for DOM changes.
  • Adds a debounced onMutation method to trigger sortListings only after page navigations.
  • Refactors and centralizes the sorting logic into a new sortListings method and removes duplicate code.
Comments suppressed due to low confidence (2)

src/lib/components/market/sort_listings.ts:75

  • Consider adding unit or integration tests for the onMutation logic to ensure that sortListings is correctly re-applied when the listing container updates on page navigation.
@debounce(500)

src/lib/components/market/sort_listings.ts:98

  • [nitpick] This complex selector appears multiple times—extract '#searchResultsRows .market_listing_row.market_recent_listing_row' into a shared constant to reduce duplication and minimize the risk of typos.
const rows = document.querySelectorAll('#searchResultsRows .market_listing_row.market_recent_listing_row');

Copy link
Collaborator

@GODrums GODrums left a comment

Choose a reason for hiding this comment

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

Thank you for noticing this bug and contributing a fix!

This already looks very good, I just added a small comment regarding the type usage.

@Step7750 Step7750 added the bug label Jun 12, 2025
Copy link
Collaborator

@GODrums GODrums left a comment

Choose a reason for hiding this comment

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

Make sure the Lint workflow doesn't fail.

Otherwise LGTM. Let's wait for @Step7750's review and this should be good to go.

Copy link
Member

@Step7750 Step7750 left a comment

Choose a reason for hiding this comment

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

Thank you for the community contribution! Few little comments but this should be good to go shortly.

@allending313 allending313 requested a review from Step7750 June 18, 2025 17:04
Copy link
Member

@Step7750 Step7750 left a comment

Choose a reason for hiding this comment

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

Thank you for putting this together and apologies for the delayed review!

@Step7750 Step7750 merged commit 8a4015d into csfloat:master Jun 30, 2025
2 checks passed
@allending313 allending313 deleted the fix/persist-sort branch June 30, 2025 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants