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

Fix initials for names containing apostrophes #29

Closed
wants to merge 2 commits into from
Closed

Fix initials for names containing apostrophes #29

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 8, 2017

So far just added the tests. I didn't manage to add a test, yet. Just adding the ' to the nonLetters doesn't seem to work, though.

Some escape characters were not necessary. This is a change demanded by standard linting.
@gr2m
Copy link
Owner

gr2m commented Apr 11, 2017

It treats the apostroph as a word separator. By default, initials(name) gives you the first letters of each word. I’d say JOD is a good default initial for John O'Doe, but then I don’t know if the convention is to ignore the middle letter for initials in that case. Is this based on feeling for yourself or is there some information on it somewhere?

@ghost
Copy link
Author

ghost commented Apr 11, 2017

@gr2m I don't have exactly proof, just some hints and the feedback of our customers.

@gr2m
Copy link
Owner

gr2m commented Apr 11, 2017

I’d like to do some more research myself first, but am currently on holidays (yeah I shouldn’t even be here right now :) Thanks for linking the discussion on stack exchange.

How about you add a workaround to your codebase for now by doing something like this

initials(name.replace(/O['’]/i, ''))

if name is John O'Doe, that workaround should return JD. Is that good enough for now?

@ghost
Copy link
Author

ghost commented Apr 12, 2017

@gr2m Oh, enjoy your holidays!
Workaround directly in our codebase won't work, since initials is a dependency of another dependency we have.

Let's see if we can find a proper solution once you return from holidays? :)

@ghost
Copy link
Author

ghost commented Jun 8, 2017

@gr2m any news on this?

@gr2m
Copy link
Owner

gr2m commented Jun 8, 2017

No, I’m sorry to let you wait for so long. I’d be okay to move forward with this. How about we add a special case for O’ and D’ and remove these before calculating the initials, so they don’t become a part of it?

There is another comment that makes a lot of sense to me from someone who has O’ in their own name:

We use COD for Christine O'Driscoll. Each capital letter gets an initial letter. Grace McDonnell would be GMD.

But that’s our current behaviour, right?

My fear is that we change behaviour here and people will complain about it.

Another option would be to allow passing a custom option which would be a function that manipulates a string before calculating initials. But the library you use of which initials is a dependency would need to support to pass in that option.

This is very tricky :/

@gr2m
Copy link
Owner

gr2m commented May 30, 2018

closing due to inactivity

@gr2m gr2m closed this May 30, 2018
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.

None yet

2 participants