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 new my markets filter, reconfigure existing filters for greater maintainability #2146

Merged
merged 13 commits into from
Aug 20, 2021

Conversation

kadenzipfel
Copy link
Contributor

Closes: #2017

  • Added a 'Ended recently' default filter to my markets
  • Reconfigured existing market filters for simpler maintainability
  • Still plan to add more advanced my markets filters, but will require some deeper work on the subgraph so this should be a useful quick fix

Testing:

  • All filters in all states work as expected, all the time
    • Trying going between every different filter and checking that the ordering is correct

@kadenzipfel kadenzipfel requested review from Mi-Lan, hexyls, pimato and Frankaus and removed request for Mi-Lan July 30, 2021 21:20
@kadenzipfel kadenzipfel changed the title Add new my markets filter, reconfigure existing filters to greater maintainability Add new my markets filter, reconfigure existing filters for greater maintainability Jul 30, 2021
Copy link
Contributor

@hexyls hexyls left a comment

Choose a reason for hiding this comment

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

When trying to filter "My Markets" + "Ended recently" + "Scalar markets" I have 1 categorical market in the list, can you reproduce?

Copy link
Contributor

@pimato pimato left a comment

Choose a reason for hiding this comment

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

Also we should probably keep track of the previously picked/default sort type when going back from market details view to market overview:

Bildschirmaufnahme.2021-08-02.um.12.31.04.mov

When trying to filter "My Markets" + "Ended recently" + "Scalar markets" I have 1 categorical market in the list, can you reproduce?

can reproduce:

Bildschirmaufnahme.2021-08-02.um.12.34.28.mov

@kadenzipfel
Copy link
Contributor Author

When trying to filter "My Markets" + "Ended recently" + "Scalar markets" I have 1 categorical market in the list, can you reproduce?

Yeah, sadly we can't query by market type yet for my markets, but I've added an subgraph issue for how to make this possible: protofire/omen-subgraph#184

@kadenzipfel
Copy link
Contributor Author

Also we should probably keep track of the previously picked/default sort type when going back from market details view to market overview:

I've set it to keep the default sort type when we go back now. It can also be set to go to the previously picked filter, but would require a little extra work... Should I make an issue for that?

@kadenzipfel kadenzipfel requested review from hexyls and pimato August 3, 2021 22:43
Copy link
Contributor

@hexyls hexyls left a comment

Choose a reason for hiding this comment

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

LGTM 🙌

@pimato
Copy link
Contributor

pimato commented Aug 4, 2021

Also we should probably keep track of the previously picked/default sort type when going back from market details view to market overview:

I've set it to keep the default sort type when we go back now. It can also be set to go to the previously picked filter, but would require a little extra work... Should I make an issue for that?

I´ve created an issue for it: #2150

Copy link
Contributor

@Mi-Lan Mi-Lan left a comment

Choose a reason for hiding this comment

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

@kadenzipfel Good job man! Tested it out works as it should and went through the code! Nice addition with the new ended recently filter. Made just one small comment.

app/src/pages/market_sections/market_home_container.tsx Outdated Show resolved Hide resolved
@kadenzipfel kadenzipfel requested a review from Mi-Lan August 16, 2021 17:58
Copy link
Contributor

@Frankaus Frankaus left a comment

Choose a reason for hiding this comment

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

All the filters work smoothly, great job and great addition on the filter under My Markets 👍

@kadenzipfel kadenzipfel requested a review from Mi-Lan August 18, 2021 16:42
Copy link
Contributor

@Mi-Lan Mi-Lan left a comment

Choose a reason for hiding this comment

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

@pimato pimato merged commit 5c9f465 into master Aug 20, 2021
@pimato pimato deleted the refactor/2017 branch August 20, 2021 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REFACTOR] Improve ending soon filter
5 participants