8000 Update Menu.php by dzidek23 · Pull Request #5464 · ChurchCRM/CRM · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update Menu.php #5464

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 10, 2020
Merged

Update Menu.php #5464

merged 2 commits into from
Nov 10, 2020

Conversation

dzidek23
Copy link

Change Persons to People in menu

What's this PR do?

Corrected menu entries from 'Persons' to People

What Issues does it Close?

Closes #5323

Where should the reviewer start?

ChurchCRM/Config/Menu/Menu.php

@dzidek23
Update Menu.php
a2a7ade 
Change Persons to People in menu
@MrClever
Copy link
Collaborator

Documentation requiring modification to reflect wording proposed in this PR:
https://github.com/ChurchCRM/CRM/search?q=persons&type=wikis

@DawoudIO
Copy link
Contributor

I think we had this topic before, I can't recall why we stayed with Persons... It was people way back... @crossan007 do you recall?

@MrClever
Copy link
Collaborator

I think we had this topic before, I can't recall why we stayed with Persons... It was people way back... @crossan007 do you recall?

The ticket #5323 refers to the recent discussion of people/persons nomenclature, although no decision seems to have been reached. Whilst "persons" is entirely correct from a native English-speaking grammatical perspective, it throws non-English speakers for a bit of a loop. I have no problem merging this PR, tweaking the documentation to reflect the use of "people" and working on aligning the codebase with the UI over time. The only people who "see" the code is really us anyway, and I think that's a cross I can bear 👍🏻🙂

8000
@MrClever
Copy link
Collaborator

@DawoudIO / @crossan007 - secondly, our own code is inconsistent on this too, even in the trio of lines @dzidek23 has addressed in the PR, so I think there is adequate precedent to merge this (from my perspective):

$peopleMenu->addSubMenu(new MenuItem(gettext("View Active Persons"), "v2/people"));
 ^^^^^^                                                   ^^^^^^^        ^^^^^^
People here                                               Persons?     People again :-/

@DawoudIO
Copy link
Contributor

made changes to POEditor to ensure we don't lose translations

@DawoudIO DawoudIO merged commit f4940e9 into ChurchCRM:master Nov 10, 2020
@DawoudIO DawoudIO added this to the 4.2.3 milestone Nov 10, 2020
@DawoudIO DawoudIO added the UI label Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crossan007 crossan007 Awaiting requested review from crossan007

@DawoudIO DawoudIO Awaiting requested review from DawoudIO

@MrClever MrClever Awaiting requested review from MrClever

Assignees
No one assigned
Labels
UI
Projects
None yet
Milestone
4.2.3
Development

Successfully merging this pull request may close these issues.

Menu - People -> View all Persons
3 participants
@dzidek23 @MrClever @DawoudIO
0