Skip to content

Initials for phone numbers #4376

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

Merged
dzearing merged 6 commits into from
Mar 28, 2018
Merged

Initials for phone numbers #4376

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
de9a4b0
Merge pull request #1 from OfficeDev/master
naitsirk Mar 26, 2018
d50184d
Adding option for calculating initials for phone numbers
naitsirk Mar 27, 2018
dd81993
Added change description
naitsirk Mar 27, 2018
55ab31b
Name change and cleanup
naitsirk Mar 27, 2018
1ca457a
Update initials-for-phonenumbers_2018-03-27-14-10.json
dzearing Mar 27, 2018
1302945
Update initials-for-phonenumbers_2018-03-27-14-10.json
dzearing Mar 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions common/changes/@uifabric/utilities/initials-for-phonenumbers_2018-03-27-14-10.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@uifabric/utilities",
"comment": "Add new option to calculate initials for phone numbers: calculateInitialsForPhoneNumber",
"type": "minor"
}
],
"packageName": "@uifabric/utilities",
"email": "[email protected]"
}
11 changes: 11 additions & 0 deletions common/changes/office-ui-fabric-react/initials-for-phonenumbers_2018-03-27-14-10.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "Add new option for Persona control to calculate initials for phone numbers: calculateInitialsForPhoneNumber",
"type": "minor"
}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
6 changes: 6 additions & 0 deletions packages/office-ui-fabric-react/src/components/DocumentCard/DocumentCard.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ export interface IDocumentCardActivityPerson {
*/
initials?: string;

/**
* Whether initials are calculated for phone numbers.
* @defaultvalue false
*/
calculateInitialsForPhoneNumber?: boolean;
Copy link
Member

@dzearing dzearing Mar 27, 2018

Choose a reason for hiding this comment

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

that's oddly specific to phone numbers. Would it be more straightforward to call it allowNumberInitials?

Also it might be a good idea to describe in the comments what special exception scenarios might be appropriate for allowing this. (Projects which start with a number, Personas which represent a phone number...)

Copy link
Author

@naitsirk-zz naitsirk-zz Mar 27, 2018

Choose a reason for hiding this comment

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

The code logic itself uses the concept of phone numbers, supporting the "ext" and "x" extension syntax in US numbers in the regular expression. The property can only consist of numbers, except for the optional "x" or "ext" at the end, after all special characters have been removed.

So without this change, "1234" will be seen as a phone number with empty initials. "1234A" will not be seen as a phone number, with initials "1", With this change, and setting calculateInitialsForPhoneNumber to true, "1234" would also get initials "1".

I agree that calculateInitialsForPhoneNumber is very specific. But allowNumberInitials doesn't quite capture the logic either, as without this fix, a contact with name "1 2A" would still get initials "12", so number only initials is already supported.

What we want to support is that values representing what we believe are phone numbers (only numbers with an optional "x" or "ext" at the end followed by more numbers) can either get empty initials or normal initials. The use case is values representing project names which are incorrectly interpreted as phone numbers (and given no initials).

I'm in no way married to the calculateInitialsForPhoneNumber name :-)

Copy link
Member

@dzearing dzearing Mar 27, 2018

Choose a reason for hiding this comment

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

ok gotcha. What about allowPhoneInitials ? A little shorter?

Copy link
Author

@naitsirk-zz naitsirk-zz Mar 27, 2018

Choose a reason for hiding this comment

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

Renamed. Also added more context to the comments. Ok to use "numbers", or should I use the more correct term "numerals"?


/**
* The background color when the user's initials are displayed.
* @defaultvalue PersonaInitialsColor.blue
Expand Down
1 change: 1 addition & 0 deletions packages/office-ui-fabric-react/src/components/DocumentCard/DocumentCardActivity.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export class DocumentCardActivity extends BaseComponent<IDocumentCardActivityPro
primaryText={ person.name }
imageUrl={ person.profileImageSrc }
initialsColor={ person.initialsColor }
calculateInitialsForPhoneNumber={ person.calculateInitialsForPhoneNumber }
role='persentation'
size={ PersonaSize.size32 }
/>
Expand Down
2 changes: 2 additions & 0 deletions packages/office-ui-fabric-react/src/components/Facepile/Facepile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export class Facepile extends BaseComponent<IFacepileProps, {}> {
imageInitials={ persona.imageInitials }
imageUrl={ persona.imageUrl }
initialsColor={ persona.initialsColor }
calculateInitialsForPhoneNumber={ persona.calculateInitialsForPhoneNumber }
primaryText={ persona.personaName }
size={ personaSize }
{ ...(getPersonaProps ? getPersonaProps(persona) : null) }
Expand All @@ -130,6 +131,7 @@ export class Facepile extends BaseComponent<IFacepileProps, {}> {
imageInitials={ persona.imageInitials }
imageUrl={ persona.imageUrl }
initialsColor={ persona.initialsColor }
calculateInitialsForPhoneNumber={ persona.calculateInitialsForPhoneNumber }
primaryText={ persona.personaName }
size={ personaSize }
{ ...(getPersonaProps ? getPersonaProps(persona) : null) }
Expand Down
6 changes: 6 additions & 0 deletions packages/office-ui-fabric-react/src/components/Facepile/Facepile.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ export interface IFacepilePersona extends React.ButtonHTMLAttributes<HTMLButtonE
*/
imageInitials?: string;

/**
* Whether initials are calculated for phone numbers.
* @defaultvalue false
*/
calculateInitialsForPhoneNumber?: boolean;

/**
* The background color when the user's initials are displayed.
* @defaultvalue [Derived from personaName]
Expand Down
2 changes: 2 additions & 0 deletions packages/office-ui-fabric-react/src/components/Persona/Persona.base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class PersonaBase extends BaseComponent<IPersonaProps, {}> {
imageAlt,
imageInitials,
initialsColor,
calculateInitialsForPhoneNumber,
presence,
primaryText,
imageShouldFadeIn,
Expand All @@ -67,6 +68,7 @@ export class PersonaBase extends BaseComponent<IPersonaProps, {}> {
imageAlt,
imageInitials,
initialsColor,
calculateInitialsForPhoneNumber,
presence,
primaryText,
imageShouldFadeIn,
Expand Down
12 changes: 12 additions & 0 deletions packages/office-ui-fabric-react/src/components/Persona/Persona.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,18 @@ describe('Persona', () => {
expect(result.text()).toEqual('45');
wrapper.unmount();

wrapper = mount(<Persona primaryText='+1 (555) 6789' />);
result = wrapper.find(STYLES.initials);
expect(result).toHaveLength(1);
expect(result.text()).toEqual('');
wrapper.unmount();

wrapper = mount(<Persona primaryText='+1 (555) 6789' calculateInitialsForPhoneNumber={true} />);
result = wrapper.find(STYLES.initials);
expect(result).toHaveLength(1);
expect(result.text()).toEqual('16');
wrapper.unmount();

wrapper = mount(<Persona primaryText='David (The man) Goff' />);
result = wrapper.find(STYLES.initials);
expect(result).toHaveLength(1);
Expand Down
6 changes: 6 additions & 0 deletions packages/office-ui-fabric-react/src/components/Persona/Persona.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ export interface IPersonaProps extends React.HTMLAttributes<PersonaBase> {
*/
imageInitials?: string;

/**
* Whether initials are calculated for phone numbers.
* @defaultvalue false
*/
calculateInitialsForPhoneNumber?: boolean;

/**
* Optional custom renderer for the initials
*/
Expand Down
3 changes: 2 additions & 1 deletion packages/office-ui-fabric-react/src/components/Persona/PersonaCoin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,11 @@ export class PersonaCoin extends React.Component<IPersonaProps, IPersonaState> {
private _onRenderInitials = (props: IPersonaProps): JSX.Element => {
let { imageInitials } = props;
const { primaryText } = props;
const { calculateInitialsForPhoneNumber } = props;

const isRTL = getRTL();

imageInitials = imageInitials || getInitials(primaryText, isRTL);
imageInitials = imageInitials || getInitials(primaryText, isRTL, calculateInitialsForPhoneNumber);

return (
imageInitials !== '' ?
Expand Down
17 changes: 17 additions & 0 deletions packages/office-ui-fabric-react/src/components/Persona/examples/Persona.Initials.Example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,30 @@ export class PersonaInitialsExample extends React.Component<any, any> {
{ ...examplePersona }
primaryText='Kat Larrson'
/>
<Persona
{ ...examplePersona }
primaryText='Annie'
/>
<Persona
{ ...examplePersona }
primaryText='Annie Lindqvist'
/>
<Persona
{ ...examplePersona }
primaryText='Annie Boyl Lindqvist'
/>
<Persona
{ ...examplePersona }
primaryText='Annie Boyl Carrie Lindqvist'
/>
<Persona
{ ...examplePersona }
primaryText='+1 (555) 123-4567 X4567'
/>
<Persona
{ ...examplePersona }
primaryText='+1 (555) 123-4567 X4567'
calculateInitialsForPhoneNumber={true}
/>
<Persona
{ ...examplePersona }
Expand Down
4 changes: 2 additions & 2 deletions packages/office-ui-fabric-react/src/components/pickers/PeoplePicker/PeoplePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class ListPeoplePicker extends MemberListPeoplePicker {
};
}

export function createGenericItem(name: string, currentValidationState: ValidationState) {
export function createGenericItem(name: string, currentValidationState: ValidationState, calculateInitialsForPhoneNumber: boolean) {
const personaToConvert = {
key: name,
primaryText: name,
Expand All @@ -61,7 +61,7 @@ export function createGenericItem(name: string, currentValidationState: Validati
};

if (currentValidationState !== ValidationState.warning) {
personaToConvert.imageInitials = getInitials(name, getRTL());
personaToConvert.imageInitials = getInitials(name, getRTL(), calculateInitialsForPhoneNumber);
}

return personaToConvert;
Expand Down
2 changes: 1 addition & 1 deletion packages/utilities/api/utilities.api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export function getFirstFocusable(rootElement: HTMLElement,
export function getId(prefix?: string): string;

// @public
export function getInitials(displayName: string | undefined | null, isRtl: boolean): string;
export function getInitials(displayName: string | undefined | null, isRtl: boolean, calculateInitialsForPhoneNumber?: boolean): string;

// @public
export function getLanguage(): string | null;
Expand Down
9 changes: 9 additions & 0 deletions packages/utilities/src/initials.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ describe('getInitials', () => {
result = getInitials('+47 12 34 56 78 (X 5678)', false);
expect(result).toEqual('');

result = getInitials('+47 12 34 56 78 (X 5678)', false, true);
expect(result).toEqual('4');

result = getInitials('47 12 34', false, true);
expect(result).toEqual('43');

result = getInitials('47 12', false, true);
expect(result).toEqual('41');

result = getInitials('1 Ext 2', false);
expect(result).toEqual('');

Expand Down
11 changes: 9 additions & 2 deletions packages/utilities/src/initials.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,22 @@ function cleanupDisplayName(displayName: string): string {
*
* @public
*/
export function getInitials(displayName: string | undefined | null, isRtl: boolean): string {
export function getInitials(
displayName: string | undefined | null,
isRtl: boolean,
calculateInitialsForPhoneNumber?: boolean,
): string {
if (!displayName) {
return '';
}

displayName = cleanupDisplayName(displayName);

// For names containing CJK characters, and phone numbers, we don't display initials
if (UNSUPPORTED_TEXT_REGEX.test(displayName) || PHONENUMBER_REGEX.test(displayName)) {
if (
UNSUPPORTED_TEXT_REGEX.test(displayName) ||
(!calculateInitialsForPhoneNumber && PHONENUMBER_REGEX.test(displayName))
) {
return '';
}

Expand Down