-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix/16227916 streaming display extension error #4071
Fix/16227916 streaming display extension error #4071
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on this one @adam-strzelec 💯
The notification regarding the Streaming payments
extension is correctly showing up if the extension is not installed
Screen.Recording.2025-01-13.at.10.12.54.mov
And no longer showing up after it is installed
Screen.Recording.2025-01-13.at.10.23.12.mov
And the Reputation
extension banner still shows up the the Create agreement
I left a small refactoring comment though and would appreciate to hear your thoughts 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice work @adam-strzelec 🙌
I would propose to refactor a bit the way we handle the link to the extension page.
What do you think if we'll extract this helper function
export const getNeededExtension = (action: Action) => {
switch (action) {
case Action.CreateDecision:
return Extension.VotingReputation;
case Action.StreamingPayment:
return Extension.StreamingPayments;
default:
return '';
}
};
from src/components/v5/common/ActionSidebar/partials/hooks.ts
and then use it here
const extensionId = getNeededExtension(selectedAction);
const extensionLink = `${COLONY_EXTENSIONS_ROUTE}/${extensionId}`;
....
<button
type="button"
onClick={() => {
navigate(extensionLink);
toggleActionSidebarOff();
}}
>
{formatText(MSG.viewExtension)}
</button>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your fast changes @adam-strzelec 🥇
I've already tested them, so I'll approve your PR 💯
There was a problem hiding this 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, working as expected, showing the message when the extension is not enabled.
52f79d8
into
feat/streaming-payments-ui
Description
Display and handle streaming payments extension error when the extension is not installed
Testing
Create
Streaming payment
action without extension installed.Diffs
Resolves #4052