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

Feature: Add additional gateway notice #7477

Merged

Conversation

kjohnson
Copy link
Member

@kjohnson kjohnson commented Aug 5, 2024

Resolves GIVE-1092

Description

This PR adds a notice about additional payment gateways to the payment gateway block.

Affects

Adds a notice in the inspector controls.

Dev note: the external link element cannot be added to the block card description directly, which only accepts a string. The implementation here is a workaround, which is documented.

Visuals

Inspector Controls for Payment Gateways block
image
Screenshot from 2024-08-05 13-27-22

Testing Instructions

Create a v3 donation form.
Go to the Build tab/screen.
Click on the Payment Gateways block.
Dismiss the notice.
Reload the form builder.

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@kjohnson kjohnson marked this pull request as ready for review August 5, 2024 17:38
@kjohnson
Copy link
Member Author

kjohnson commented Aug 5, 2024

@glaubersilva glaubersilva self-requested a review August 5, 2024 17:51
@kjohnson
Copy link
Member Author

kjohnson commented Aug 6, 2024

@kjohnson
Copy link
Member Author

kjohnson commented Aug 6, 2024

Copy link
Contributor

@glaubersilva glaubersilva left a comment

Choose a reason for hiding this comment

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

@kjohnson Looks good to me, I just left a comment related to a small change.

Also, I noticed on the Zeplin Design that the offline instructions are displayed before the notice instead of after, which makes the implementation from this PR slightly different from the original design - check the attached screenshot for reference.

Honestly, I don't know if it should be a concern. So, I would like to confirm with @jdghinson if it's ok as it is currently or if we need to change it.

image

@kjohnson
Copy link
Member Author

kjohnson commented Aug 8, 2024

@glaubersilva thanks. I didn't have the Offline Donations showing when I added this. I'll take a look.

@kjohnson
Copy link
Member Author

@glaubersilva it seems that the Offline Donations settings are added by filtering the Block's Edit component, so there isn't a way to control this ordering from within the Payment Gateways block.

We could inject this notice as a filter, which is what I'll try next.

@kjohnson
Copy link
Member Author

The order issue is actually due to the use of a Higher Order Component to append the Offline Donation inspector controls, where as the filter is only used to render the block itself.

@kjohnson
Copy link
Member Author

@glaubersilva Ok, I was able to re-position the notice using a filter and a higher order component, see 15ca140.

image

@glaubersilva
Copy link
Contributor

@kjohnson Thanks for the changes, this seems more with the design now. However, to compile the files on my local machine I have to delete lines a couple of unused imports on the FormBuilder/resources/js/form-builder/src/blocks/fields/payment-gateways/Edit.tsx file.

Besides that, it's all good. So, I'll approve the PR to advance things, but remember to remove this not used imports before sending it to QA. =)

Nice work!

image

Copy link
Contributor

@glaubersilva glaubersilva left a comment

Choose a reason for hiding this comment

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

ThumbsThumbsUpKidGIF

@kjohnson
Copy link
Member Author

Thanks, @glaubersilva. I removed those unused imports, see d1e3cde.

@kjohnson
Copy link
Member Author

Copy link
Member

@rickalday rickalday left a comment

Choose a reason for hiding this comment

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

Passed manual QA tests.

@glaubersilva glaubersilva merged commit 18ec8d3 into develop Aug 30, 2024
20 checks passed
@glaubersilva glaubersilva deleted the feature/form-builder-additional-gateways-notice-1092 branch August 30, 2024 16:33
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.

4 participants