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(#385): portfolio positions #386

Merged
merged 9 commits into from
Mar 10, 2025

Conversation

VanishMax
Copy link
Contributor

Closes #385

In this PR:

  • Major refactoring of the logic related to positions and transactions. Previously, positions table relied on the usePathMetadata hook to get base and quote assets and later filter positions based on this. Also, it relied on the positionsStore that was difficult to scale and reuse in different pages. During this refactoring, I split the store into independent functions and moved them to the new entities folder based on feature-sliced conventions.
  • Moved the component from Table UI component to TableCell, and from table to grids – it allowed simpler multi-order positions rendering
  • Reused positions table in the portfolio page
  • Added infinite scroll to the table
image

@VanishMax VanishMax requested a review from a team March 7, 2025 14:24
@VanishMax VanishMax self-assigned this Mar 7, 2025
Comment on lines +16 to +21
/** Helper function that allows breaking early when using stream responses */
async function* limitAsync<T>(
iterable: AsyncIterable<T>,
limit: number,
offset: number,
): AsyncIterable<T> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this function replaces basic Array.fromAsync and breaks early if the limit is reached, thus reducing calculations

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

import { bech32mPositionId } from '@penumbra-zone/bech32m/plpid';
import { queryClient } from '@/shared/const/queryClient';
import { AddressIndex } from '@penumbra-zone/protobuf/penumbra/core/keys/v1/keys_pb';

const BASE_LIMIT = 20;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if you want to play with the infinite scroll, reduce this limit

Comment on lines +204 to +210
export const getDisplayPositions = ({
positions,
getMetadataByAssetId,
asset1Filter,
asset2Filter,
stateFilter,
}: GetDisplayPositionsArgs): DisplayPosition[] => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored the arguments of this function – now it accepts filters. By default, returns all positions

</div>

{(isLoading ? loadingArr : sortedPositions).map((position, index) => (
<Fragment key={`${position.idString}${index}`}>
Copy link
Contributor

@JasonMHasperhoven JasonMHasperhoven Mar 7, 2025

Choose a reason for hiding this comment

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

Nitpick: You don't need to do this btw, keys only have to be unique for each loop. So index is enough here incase we have multiple positions with the same idString.

@VanishMax VanishMax merged commit afc4c85 into feat/#381-history-tab Mar 10, 2025
3 checks passed
@VanishMax VanishMax deleted the feat/#385-portfolio-positions branch March 10, 2025 05:30
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.

2 participants