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

feat(ui): save sort state in redux #3415

Merged
merged 8 commits into from
Aug 31, 2024
Merged

feat(ui): save sort state in redux #3415

merged 8 commits into from
Aug 31, 2024

Conversation

markphelps
Copy link
Collaborator

@markphelps markphelps commented Aug 28, 2024

Re: #3413

2024-08-31 12 01 35

Moves sorting state of tables to redux so that it persists between pages

TODO

  • Segments Table
  • Anywhere else we allow sorting in tables will do this if requested by users later

Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps changed the title feat: Save sort state feat(ui): save sort state in redux Aug 29, 2024
Signed-off-by: Mark Phelps <[email protected]>
Copy link

codecov bot commented Aug 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.65%. Comparing base (507170d) to head (3a9b174).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3415   +/-   ##
=======================================
  Coverage   64.65%   64.65%           
=======================================
  Files         174      174           
  Lines       13907    13907           
=======================================
  Hits         8992     8992           
  Misses       4227     4227           
  Partials      688      688           
Flag Coverage Δ
unittests 64.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@erka
Copy link
Collaborator

erka commented Aug 29, 2024

We need to change API to pass the order param because of pagination.

@markphelps
Copy link
Collaborator Author

We need to change API to pass the order param because of pagination.

How do you mean? The pagination still happens client side as well

@erka
Copy link
Collaborator

erka commented Aug 29, 2024

Client fetches the limited number of flags/segments, right? If backend has no knowledge of ordering that may lead to unexpected results if I don't miss something.

@erka
Copy link
Collaborator

erka commented Aug 29, 2024

Client fetches the limited number of flags/segments, right? If backend has no knowledge of ordering that may lead to unexpected results if I don't miss something.

My bad... it looks like client fetches all the flags at once. I looked at tests where limit is set

@markphelps
Copy link
Collaborator Author

Yeah we would eventually like to move pagination/sorting/etc to server side (call the API) but this has implications with the search box that pops up if you have more than 15 flags (I think).

Currently search is done client side too, which it can do because it pulls down all flags at once. If we moved to server side pagination we would have to come up with a different solution for search, like an API endpoint

@markphelps markphelps marked this pull request as ready for review August 31, 2024 16:03
@markphelps markphelps requested a review from a team as a code owner August 31, 2024 16:03
Copy link
Collaborator

@erka erka left a comment

Choose a reason for hiding this comment

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

nice

Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps added the automerge Used by Kodiak bot to automerge PRs label Aug 31, 2024
@kodiakhq kodiakhq bot merged commit ce7f77c into main Aug 31, 2024
63 checks passed
@kodiakhq kodiakhq bot deleted the save-sort-state branch August 31, 2024 20:58
markphelps added a commit that referenced this pull request Aug 31, 2024
* 'main' of https://github.com/flipt-io/flipt:
  feat(ui): save sort state in redux (#3415)
  fix(authz): added a helper function to the policy engine to simplify (#3419)
  chore: upgrade formbricks lib; reword banner and onboarding (#3414)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Used by Kodiak bot to automerge PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants