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

Allow change username server-side error messages to be html #2805

Merged
merged 10 commits into from Mar 26, 2018

Conversation

3 participants
@notbakaneko
Contributor

notbakaneko commented Mar 26, 2018

fixes #2793 and #2798

Also move the messages to translations.

} elseif ($remaining->h > 0) {
return ["This username will be available for use in <strong>{$remaining->h}</strong> hours."];
return [trans_choice('model_validation.user.username_available_in_hours', $remaining->hours)];

This comment has been minimized.

@nanaya

nanaya Mar 26, 2018

Collaborator

The property name used between here and the check doesn't match ಠ_ಠ

Or more like ->hours doesn't exist.

@@ -88,11 +88,28 @@
'invalid_email' => "Doesn't seem to be a valid email address.",
'too_short' => 'New password is too short.',
'unknown_duplicate' => 'Username or email address already used.',
'username_available_in_days' => 'This username will be available for use in <strong>:count</strong> day.|This username will be available for use in <strong>:count</strong> days.',
'username_available_in_hours' => 'This username will be available for use in <strong>:count</strong> hour.|This username will be available for use in <strong>:count</strong> hours.',

This comment has been minimized.

@nanaya

nanaya Mar 26, 2018

Collaborator

The <strong> part can probably be extended to include its unit as well (or just removed because current design has <strong> wrapping the whole thing). And then the hours and days version can be merged or something.

@@ -88,11 +88,28 @@
'invalid_email' => "Doesn't seem to be a valid email address.",
'too_short' => 'New password is too short.',
'unknown_duplicate' => 'Username or email address already used.',
'username_available_in_days' => '{1} This username will be available for use in :count day.|[2,*] This username will be available for use in :count days.',
'username_available_in_hours' => '{1} This username will be available for use in :count hour.|[2,*] This username will be available for use in :count hours.',

This comment has been minimized.

@nanaya

nanaya Mar 26, 2018

Collaborator

does it actually do anything? Looks the same as without explicit numbers.

This comment has been minimized.

@notbakaneko

notbakaneko Mar 26, 2018

Contributor

it makes the pl translation use the en rules when untranslated

This comment has been minimized.

@nanaya

nanaya Mar 26, 2018

Collaborator

looks more like symfony/laravel bug ಠ_ಠ also missing 0.

This comment has been minimized.

@notbakaneko

notbakaneko Mar 26, 2018

Contributor

we won't need 0 yet :trollface:

This comment has been minimized.

@nanaya

nanaya Mar 26, 2018

Collaborator

I'd rather "fix" the trans_choice function so we don't need to do this for everything.

notbakaneko added some commits Mar 26, 2018

@nanaya

nanaya approved these changes Mar 26, 2018

@nanaya nanaya merged commit 809efa3 into ppy:master Mar 26, 2018

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@notbakaneko notbakaneko deleted the notbakaneko:fix/store-username-formatted-string branch Mar 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment