search

kokarn / tarkov-tools

https://tarkov-tools.com
MIT License
70 stars 44 forks source link

Add the ability to filter ammo chart by my trader levels or price #155

Open Reithan opened 2 years ago

Reithan commented 2 years ago

Since tarkov tools has my trader levels set in settings, and it has market data from it's API, it would be awesome to be able to set a max price, or max trader level for ammos and be able to see what ammos I should be considering.

kokarn commented 2 years ago

Wow, that's a great suggestion! Can't believe I haven't thought of this sooner.

AnyFinCanHappen commented 2 years ago

Hello @kokarn can I work on this issue? Unless @Reithan is already working on this one.

Also, here's my idea on implementing this. Since we are having multiple filter categories, we can have a VictoryLegend instance for each category. So for our case we would have 3 legends, "Filter by caliber", "Filter by Trader Level", and "Filter by Price".

If we go ahead with using three VictoryLegend instances, then we could have the legends be more horizontally shaped and ordered to be on top of the graph.

Mock: The three red boxes represent the legends tarkov_tool_idea

Let me know what you think!

Also not really sure what kind of form to use for "Filter by Price", maybe just two input text?

Reithan commented 2 years ago

Filter by trader could just be a toggle like it is for other views. Possible add a toggle to hide ammo that's banned on flea as well.

Filter by price could just be a min-max text entry thing.

I think leaving filter by caliber as-is could be reasonable.

Reithan commented 2 years ago

Oh, as to the question if I'm currently working on this, no I am not, @AnyFinCanHappen

AnyFinCanHappen commented 2 years ago

@Reithan thanks for the feedback,

Im fine with leaving "filter by caliber" as it is. If we do that I'm not really sure where to put the filter by traders and price range. So for now I just work on the functionality for each of the filters and wait on @kokarn opinion on where we should put the filters.

Also some things to note: I'm not sure if we can have custom components in a VictoryContainer So we may not be able to place the price range and trader filter along side the caliber filter.

kokarn commented 2 years ago

Very cool that you want to work on this @AnyFinCanHappen !

I'n general I'm fine with you trying out what you think works best.

If you want my thinking I was thinking more about using the Filter that already exists and putting on top. I've had trouble styling the Legend in the past and I think people want to use all filters combined in the same way they do on other pages.

Reithan commented 2 years ago

Wanted to revisit one thing I said: I said min-max price range, but tbh, I can't think of a super solid reason to want to filter to a min price? Maybe max price by itself would be enough? I could definitely be wrong though. If you think min would be valuable, it shouldn't be too hard to take or leave it when/if you add max price.

kokarn commented 2 years ago

I agree, no need for "min" price which also makes the implementation match the rest of the site 👍

AnyFinCanHappen commented 2 years ago

@kokarn sorry for the dead silence. Something came up IRL so, I couldn't really make any progress on this. If needed, someone can take my spot for this issue.

Also, will take a look at #157 when I can :)

kokarn commented 2 years ago

No worries at all! Stuff happens all the time :) Just let me know if you need help and/or support with anything, whenever you get the opportunity :)