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: Select payment methods as default in the payment settings #1159

Merged
merged 6 commits into from
Jul 21, 2022

Conversation

SGrueber
Copy link
Collaborator

@SGrueber SGrueber commented May 11, 2022

PR Type

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no API changes)
[ ] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[ ] Application / infrastructure changes
[ ] Other:

What Is the Current Behavior?

Only payment instruments like credit cards can be set as preferred payment methods in the myAccount payment settings.

Issue Number: Closes #

What Is the New Behavior?

Also payment methods that don't require parameters like invoice can be set as default payment method.

Does this PR Introduce a Breaking Change?

[ ] Yes
[x] No

Other Information

AB#76608

@SGrueber SGrueber marked this pull request as draft May 11, 2022 13:07
@SGrueber SGrueber self-assigned this May 11, 2022
@SGrueber SGrueber added the feature New feature or request label May 11, 2022
@SGrueber SGrueber added this to the 2.4 milestone May 11, 2022
@SGrueber SGrueber force-pushed the feature/account_payment_enhancements branch from e6bfb85 to 8d87620 Compare May 11, 2022 14:04
@SGrueber SGrueber changed the title feat: All payment methods can be selected as default in the payment settings feat: Select payment methods as default in the payment settings May 13, 2022
@SGrueber SGrueber force-pushed the feature/account_payment_enhancements branch from eed5c3c to ea6782c Compare May 16, 2022 06:48
@SGrueber SGrueber requested review from M-Behr and iwiederhold May 16, 2022 07:08
@SGrueber SGrueber marked this pull request as ready for review May 16, 2022 08:58
Copy link

@iwiederhold iwiederhold left a comment

Choose a reason for hiding this comment

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

  • Strange behavior when loading. Please check this process.
  • Different panel heights for single line texts at the Preferred payment panel.
  • Could we use the same spaces as in the checkout payment panels?

@SGrueber SGrueber force-pushed the feature/account_payment_enhancements branch from 201c329 to b696067 Compare May 16, 2022 15:16
@SGrueber SGrueber requested a review from iwiederhold May 17, 2022 08:56
iwiederhold
iwiederhold previously approved these changes May 17, 2022
Copy link

@iwiederhold iwiederhold left a comment

Choose a reason for hiding this comment

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

Nice! And good cooperation!

@Eisie96 Eisie96 self-requested a review May 18, 2022 06:37
Copy link

@M-Behr M-Behr left a comment

Choose a reason for hiding this comment

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

It works very well 👍 Yesterday we decided together to make small changes:

  • Show another success message, when selecting "No preferred payment method". Something like: "You have no longer a preferred payment method selected."
  • Replace trash icons with "Delete" link for a consistent UI between Checkout and My Account

@SGrueber SGrueber force-pushed the feature/account_payment_enhancements branch from 7ce2cba to 4cfd0f6 Compare May 20, 2022 09:37
@SGrueber SGrueber requested review from mglatter and M-Behr May 20, 2022 09:39
M-Behr
M-Behr previously approved these changes May 20, 2022
Copy link

@M-Behr M-Behr left a comment

Choose a reason for hiding this comment

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

Perfect, thank you!

"account.payment.sepa_accepted_on": "Accepté le",
"account.payment.sepa_mandate_Reference": "Référence du mandat",
"account.payment.standard_methods": "Méthodes de paiement standard",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please have Nicolas review the whole FR part. I have the feeling that this must be "Méthodes de paiement standards" but he will be sure about the correct grammar. ;-)

Choose a reason for hiding this comment

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

"modes de paiement" is the correct wording

Copy link
Contributor

@mglatter mglatter left a comment

Choose a reason for hiding this comment

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

Please ask Nicolas for review of fr_FR.json

@SGrueber SGrueber force-pushed the feature/account_payment_enhancements branch from 4cfd0f6 to 12967fd Compare May 24, 2022 07:35
@jometzner jometzner modified the milestones: 2.4, 3.0 May 30, 2022
Copy link

@NicolasGrizard NicolasGrizard left a comment

Choose a reason for hiding this comment

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

line 290 of fr_FR.json should be "Modes de paiement standard"

@MaxKless MaxKless self-requested a review July 20, 2022 10:08
@MaxKless MaxKless force-pushed the feature/account_payment_enhancements branch from 05ea213 to a1e41ad Compare July 20, 2022 10:16
@MaxKless MaxKless force-pushed the feature/account_payment_enhancements branch from a1e41ad to 2590421 Compare July 20, 2022 12:53
@MaxKless MaxKless merged commit 62718e9 into develop Jul 21, 2022
@MaxKless MaxKless deleted the feature/account_payment_enhancements branch July 21, 2022 08:39
shauke pushed a commit that referenced this pull request Jul 21, 2022
Co-authored-by: max.kless@googlemail.com <max.kless@googlemail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants