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

POC - Settings tweaks #49

Open
wants to merge 11 commits into
base: trunk
Choose a base branch
from
Open

POC - Settings tweaks #49

wants to merge 11 commits into from

Conversation

cyl3x
Copy link
Contributor

@cyl3x cyl3x commented Dec 2, 2024

Goals:

  • Reduce complexity and components
  • Increase readability
  • Simplify by abstract common behaviour - even label & helpText snippets
  • No longer override SystemConfigController - but still register webhook and show errors about it
  • validate-api-credentials shouldn't throw errors, as 401 will trigger a retry-request

The idea:

  • Separate settings and payment method card
  • Create shared states for system-config (swagPayPalSettings store) and merchant-information (swagPayPalMerchantInformation store)
  • Onboarding is a component and reduced to a button
  • settings tabs are actual routes with own views
  • swag-paypal-settings-wrapper creates common fields like switch, single-select and text-field (but nothing specific for a setting)
  • SettingsSaver will save system settings and reregisteres the webhook + check if changed credentials are valid
  • SettingsController will use the SettingsSaver for an own system-config save route

To check:

  • Should a store be capable of loading and saving itself instead of using mixins?
  • Is swag-paypal-method a suitable name?

@cyl3x cyl3x added Feature Request Work in progress Proposal is work in progress labels Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Work in progress Proposal is work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant