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: add teams filtration for streaming payments page #4057

Merged

Conversation

adam-strzelec
Copy link
Contributor

@adam-strzelec adam-strzelec commented Jan 9, 2025

Description

Add filtration for streaming payments page

Testing

Open Streaming payments page

Diffs

New stuff
Screenshot 2025-01-09 at 09 32 59

Resolves #4021

@adam-strzelec adam-strzelec requested a review from a team as a code owner January 9, 2025 08:36
@adam-strzelec adam-strzelec self-assigned this Jan 9, 2025
@AdrianDudko AdrianDudko linked an issue Jan 9, 2025 that may be closed by this pull request
Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

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

Nice job, all seems to be working as expected!

A team with an active payment:
Screenshot 2025-01-10 at 18 06 24

A team without:
Screenshot 2025-01-10 at 18 06 29

With all teams selected:
Screenshot 2025-01-10 at 18 06 35

Copy link
Contributor

@mmioana mmioana left a comment

Choose a reason for hiding this comment

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

Awesome work on this one @adam-strzelec 🥇

Previously created 2 active streaming payments and one that has not yet started in Rocinate

View for All teams
Screenshot 2025-01-13 at 10 33 33
View for General
Screenshot 2025-01-13 at 10 33 36
View for Rocinate
Screenshot 2025-01-13 at 10 33 41
View for Andromeda where there are no streams
Screenshot 2025-01-13 at 10 35 56

I left a small refactoring comment so we keep it consistent as much as possible with the other places where we use the team filtering 💯

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we please use here ContentWithTeamFilter instead of TeamFilter?

So we could refactor this as

<ContentWithTeamFilter>
      <StatsCards
        streamingPerMonth={totalLastMonthStreaming}
        totalActiveStreams={activeStreamingPayments}
        totalStreamed={totalStreamed}
        unclaimedFounds={totalFunds.totalAvailable}
        prefix={currencySymbolMap[currency]}
        suffix={currency}
      />
    </ContentWithTeamFilter>

@adam-strzelec adam-strzelec requested a review from mmioana January 13, 2025 12:53
@iamsamgibbs
Copy link
Contributor

@adam-strzelec There is already a ContentWithTeamFilter component here: src/components/v5/frame/ContentWithTeamFilter/ContentWithTeamFilter.tsx Although it looks like some of the classnames are different. My preference would be to keep the StatsCards wrapped in a div to add the extra spacing rather than creating a second ContentWithTeamFilter component.

Copy link
Contributor

@mmioana mmioana left a comment

Choose a reason for hiding this comment

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

Hey @adam-strzelec thank you for your super-fast changes! 💯

However I had in mind re-using the existing ContentWithTeamFilter component as there is no value into creating a new one 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we please use the existing component ContentWithTeamFilter from src/components/v5/frame/ContentWithTeamFilter/ContentWithTeamFilter.tsx?

And if we really need the distance between the filter and content to be of 8, we could refactor the component as in

import React, { type FC, type PropsWithChildren } from 'react';

import TeamFilter from '~v5/shared/TeamFilter/TeamFilter.tsx';

const displayName = 'v5.frame.ContentWithTeamFilter';

interface ContentWithTeamFilterProps extends PropsWithChildren {
  className?: string;
}

const ContentWithTeamFilter: FC<ContentWithTeamFilterProps> = ({
  className = 'pb-6',
  children,
}) => {
  return (
    <div className="flex flex-col">
      <div className={className}>
        <TeamFilter />
      </div>
      {children}
    </div>
  );
};

ContentWithTeamFilter.displayName = displayName;
export default ContentWithTeamFilter;

/>
</div>
</>
<ContentWithTeamFilter>
Copy link
Contributor

Choose a reason for hiding this comment

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

And then pass here the className='pb-8'

@adam-strzelec adam-strzelec requested a review from mmioana January 13, 2025 13:44
@adam-strzelec
Copy link
Contributor Author

@mmioana @iamsamgibbs Right, I didn't notice that this component already exists 🤦‍♂️

Copy link
Contributor

@mmioana mmioana left a comment

Choose a reason for hiding this comment

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

Thanks for your continuous effort @adam-strzelec 🙌 everything works as expected

Screen.Recording.2025-01-13.at.16.07.04.mov

Copy link
Member

@arrenv arrenv left a comment

Choose a reason for hiding this comment

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

@adam-strzelec Nice one, looks and works great!!

image

image

Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

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

Thanks for the change! This is good now.

Screenshot 2025-01-14 at 17 40 13

Although I have now noticed that the totals on this page include inactive payments from previous installations of the extension so this will need addressing as a later issue.

@adam-strzelec adam-strzelec merged commit ab28bf5 into feat/streaming-payments-ui Jan 15, 2025
3 of 5 checks passed
@adam-strzelec adam-strzelec deleted the feat/16220654-teams-filtration branch January 15, 2025 10:07
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.

Streaming payments team filter
4 participants