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

Add support for selecting league for Timeless Jewel search #7028

Conversation

Peechey
Copy link
Contributor

@Peechey Peechey commented Dec 13, 2023

Fixes #6906

Description of the problem being solved:

Adds a trade league dropdown to the timeless jewel search. Given the user does not interact with the dropdown, the current behavior is kept and we create the trade url without the league name which defaults to the current base league. The dropdown is setup to show this same league on load so as to avoid confusion. Once any value is selected, that league specifically is added to the url.

The realm is hardcoded to PC, which is the same behavior we have today, but we need to supply a realm to get leagues. A lot of this code is similar to what's in TradeQuery.lua (~line 350) but I wasn't sure if there was a clean way to strip that out to reuse. I have it sorting to have Affliction be at index 1, and maybe we'd also like that for the Trade popup in the ItemsTab.

I also added "Solo" in the filter in TradeQueryRequests so "Solo Self-Found" is not shown for this dropdown nor the Trade for Items popup.

Considerations:

  • The way I coded it to put "Affliction" first could probably be improved
  • It makes a call out for realms every single time you open the timeless jewel search, maybe it should be moved somewhere in TreeTab to happen only once?

Steps taken to verify a working solution:

  • Default state: open the search, select some nodes, get search results and copy trade url without interacting with the dropdown. Confirm a PC Affliction trade search link is created.
  • Do the same as above but select any league in the dropdown, confirm a PC selected league trade search link is generated.
  • Manually break the HTTP call to generate an error, and then confirm that a url is still generated with the default behavior (empty league in url -> PC Affliction)

After screenshot:

image

image

@Peechey Peechey added the enhancement New feature, calculation, or mod label Dec 13, 2023
@LocalIdentity LocalIdentity merged commit 88ef4f5 into PathOfBuildingCommunity:dev Dec 17, 2023
@Peechey Peechey deleted the feature/timeless_jewel_league_select branch January 4, 2024 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, calculation, or mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeless Jewel - Trade league select
2 participants