Skip to content

SL-350 Settings copy & link fixes (SL-365/368/369/372/373)#333

Merged
justelis22 merged 3 commits into
feature/react-admin-settingsfrom
SL-350/settings-copy
Jul 10, 2026
Merged

SL-350 Settings copy & link fixes (SL-365/368/369/372/373)#333
justelis22 merged 3 commits into
feature/react-admin-settingsfrom
SL-350/settings-copy

Conversation

@justelis22

Copy link
Copy Markdown
Collaborator

Part of SL-350 settings redesign. Phase 1 — copy & link changes (no logic).

Tickets

  • SL-368 — deep-link the Saferpay Fields library docs anchor.
  • SL-369 — reword the 3DS liability-shift setting labels.
  • SL-373 — clarify Email Sending toggle descriptions (fixes fixed issue with paypal beeins used as transacion method #3 duplicating Readme #1); merchant-email pointer folded into the section description.
  • SL-372 — correct the misleading "Hosted field style" info box.
  • SL-365 — credentials hint with an environment-based Saferpay Backoffice link + JSON API Basic auth docs link.

Notes

  • Strings stay translatable ($this->module->l(...) / t()); links use target="_blank" rel="noopener noreferrer".
  • Copy quoting keeps its html_entity_decode(..., ENT_QUOTES, 'UTF-8') wrapper.
  • .tsx changes (SL-368, SL-365): the compiled bundle (views/js/admin/dist) is gitignored and built in CI.

Verification

php -l clean; settings-app tsc && vite build passes.

- SL-368: deep-link Saferpay Fields library docs anchor
- SL-369: reword 3DS liability-shift setting labels
- SL-373: clarify Email Sending toggle descriptions; fold merchant-email pointer into section description
- SL-372: correct misleading Hosted field style info box
- SL-365: add credentials hint with env-based Backoffice link + JSON API Basic auth docs link

Compiled bundle (views/js/admin/dist) is gitignored and built in CI.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a credentials generation hint in the UI and updates several translation strings for improved clarity regarding payment behavior and email notifications. The review suggests refactoring the internationalization approach by using placeholders instead of splitting sentences into multiple keys to better support localization. Additionally, it recommends updating the terminology for the 'API Credentials' tab to ensure consistency and avoid user confusion.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +112 to +115
'credentialsHintPrefix' => $this->module->l('You can generate your API credentials inside the', self::FILE_NAME),
'credentialsBackofficeLinkText' => $this->module->l('Saferpay Backoffice', self::FILE_NAME),
'credentialsHintMiddle' => html_entity_decode($this->module->l('under Settings > JSON API Basic authentication.', self::FILE_NAME), ENT_QUOTES, 'UTF-8'),
'credentialsMoreInfoLinkText' => $this->module->l('More information', self::FILE_NAME),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Splitting a sentence into multiple translation keys (credentialsHintPrefix, credentialsHintMiddle, etc.) is an internationalization (i18n) anti-pattern. Different languages have different word orders and grammatical structures. Translators will not be able to translate these fragmented strings accurately or naturally.

Instead, define a single translation string with placeholders for the links, and then parse/split the string in the React component to render the links.

            'credentialsHint' => html_entity_decode($this->module->l('You can generate your API credentials inside the [backoffice_link] under Settings > JSON API Basic authentication. [more_info_link]', self::FILE_NAME), ENT_QUOTES, 'UTF-8'),
            'credentialsBackofficeLinkText' => $this->module->l('Saferpay Backoffice', self::FILE_NAME),
            'credentialsMoreInfoLinkText' => $this->module->l('More information', self::FILE_NAME),

Comment on lines +158 to +163
<p className="sp-mb-0">
{t('credentialsHintPrefix')}{' '}
<a href={backofficeUrl} target="_blank" rel="noopener noreferrer" className="sp-text-[#294e57] sp-underline hover:sp-no-underline">{t('credentialsBackofficeLinkText')}</a>{' '}
{t('credentialsHintMiddle')}{' '}
<a href={jsonApiBasicAuthDocsUrl} target="_blank" rel="noopener noreferrer" className="sp-text-[#294e57] sp-underline hover:sp-no-underline">{t('credentialsMoreInfoLinkText')}</a>
</p>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To support the single translation string pattern and avoid sentence splitting, split the translated string by the placeholders and map them to the corresponding React link elements.

              <p className="sp-mb-0">
                {t('credentialsHint').split(/(\[backoffice_link\]|\[more_info_link\])/).map((part, index) => {
                  if (part === '[backoffice_link]') {
                    return (
                      <a key={index} href={backofficeUrl} target="_blank" rel="noopener noreferrer" className="sp-text-[#294e57] sp-underline hover:sp-no-underline">
                        {t('credentialsBackofficeLinkText')}
                      </a>
                    )
                  }
                  if (part === '[more_info_link]') {
                    return (
                      <a key={index} href={jsonApiBasicAuthDocsUrl} target="_blank" rel="noopener noreferrer" className="sp-text-[#294e57] sp-underline hover:sp-no-underline">
                        {t('credentialsMoreInfoLinkText')}
                      </a>
                    )
                  }
                  return part
                })}
              </p>

return [
'emailSending' => $this->module->l('Email Sending', self::FILE_NAME),
'emailSendingDescription' => $this->module->l('Configure which emails are sent during the payment process.', self::FILE_NAME),
'emailSendingDescription' => $this->module->l('Configure which emails are sent during the payment process. Merchant notifications sent by Saferpay use the Merchant Email(s) field on the Credentials tab.', self::FILE_NAME),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The description refers to the 'Credentials tab', but the actual tab name is 'API Credentials' (defined as tabApiCredentials). To avoid merchant confusion, it is better to refer to it as the 'API Credentials' tab.

            'emailSendingDescription' => $this->module->l('Configure which emails are sent during the payment process. Merchant notifications sent by Saferpay use the Merchant Email(s) field on the API Credentials tab.', self::FILE_NAME),

Justas added 2 commits July 10, 2026 11:33
…ab name

Address review feedback:
- Combine the split credentials-hint keys into one translatable string with
  [backoffice_link]/[more_info_link] placeholders, parsed in the component (avoids the
  i18n sentence-splitting anti-pattern).
- Refer to the 'API Credentials' tab by its actual name in the Email Sending description.
@justelis22 justelis22 merged commit 5dda309 into feature/react-admin-settings Jul 10, 2026
1 of 3 checks passed
@justelis22 justelis22 deleted the SL-350/settings-copy branch July 10, 2026 13:45
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.

1 participant