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

Fixed #13495 added setting for name order #13496

Merged
merged 12 commits into from
Aug 22, 2023
Merged

Conversation

snipe
Copy link
Owner

@snipe snipe commented Aug 21, 2023

This should fix #13495, where a Korean user mentioned that in Korea, names are displayed last name first. This adds a setting in Settings > Localization that will let the admin decide whether names should be presented last name first or first name first. I also found a method that I don't think we use anymore.

Screenshot 2023-08-21 at 8 17 41 PM Screenshot 2023-08-21 at 8 17 58 PM Screenshot 2023-08-21 at 8 18 13 PM Screenshot 2023-08-21 at 8 18 32 PM

@what-the-diff
Copy link

what-the-diff bot commented Aug 21, 2023

PR Summary

  • Modification to Settings Controller
    This change helps to store the preferred user name display format in the settings.

  • Adjustment in Users Transformer
    Now, the system is utilizing the getFullNameAttribute() method to generate the full name of users, empowering flexibility in the name display format.

  • Update in User Model
    Improved the getFullNameAttribute() method to return names based on the preferred display format stored in the settings.

  • UserPresenter Adjustment
    The system now uses the getFullNameAttribute() method instead of the fullName() method for better naming consistency.

  • Database Migration
    Introduced a new column name_display_format in the settings table, allowing a place to store users' name display preferences.

  • Translation String Additions
    Addition of new translation strings to potentially enhance international user experience with custom name display formats.

  • Form Macro Update
    Introduced the Form::name_display_format macro for enhanced name form creation and easier utilization.

  • UI Changes in Account Profile and Default Layout
    Adjustments were made to the layout of the language selection dropdown and the name display in the dropdown menu for logged-in users to make it more user friendly.

  • Addition in Settings View
    Added a field for name display format preference in the settings view allowing users to change the name display format as per their preference.

Copy link
Collaborator

@spencerrlongg spencerrlongg left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Collaborator

@marcusmoore marcusmoore left a comment

Choose a reason for hiding this comment

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

A couple minor notes but nothing to hold this back. Looks good 😄

@@ -24,7 +24,7 @@ public function transformUser(User $user)
$array = [
'id' => (int) $user->id,
'avatar' => e($user->present()->gravatar),
'name' => e($user->first_name).' '.e($user->last_name),
'name' => e($user->getFullNameAttribute()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sure you already know this but because this is an accessor on the user model we can do $user->full_name instead of having to call the method.

Is calling the method an intentional pattern that I should start following?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not sure what you mean here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The get*Attribute() syntax tells Laravel to allow it to be accessed like a "normal" property so you could do $user->full_name.

I'm 99% sure there is no benefit doing it that way but I normally don't see the full get*Attribute() call in code.

resources/lang/en/general.php Outdated Show resolved Hide resolved
@snipe snipe merged commit a48762c into develop Aug 22, 2023
@snipe snipe deleted the features/setting_for_name_order branch August 22, 2023 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants