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

Is not working #7

Closed
dschinkel opened this issue Oct 21, 2020 · 5 comments
Closed

Is not working #7

dschinkel opened this issue Oct 21, 2020 · 5 comments

Comments

@dschinkel
Copy link

it's not truncating anything

<TruncateString text="abdctafdafdsfsdddd" truncateAt={10} />

nothing happens. It renders the full string.

@albinotonnina
Copy link
Owner

You're trying to truncate a word. There are no spaces in there. This library truncate strings, meaning text with spaces.
Please read the Readme.

@dschinkel
Copy link
Author

dschinkel commented Oct 21, 2020

eh, ok thanks.

Please read the Readme.

Was going to leave it at OK but...that's pretty annoying for you to say something so snarky TBH because:

Your README talks nothing about spaces being a requirement for this behavior to work. While your examples show spaces, it's pretty easy to infer that a space is just part of chars that you can cut. Hence the actual helper itself TruncateString actually to me infers spaces is Part of a string! So It needs to be updated then to be very clear if that's not obvious.

Also had read the Readme already before posting this, and the fact that your future features show:
min start and end characters option?

inferred to me again that whatever is in that string, whether there are space are not is a string!

So really your lib is missing a use case / edge case with a string, a string without spaces, not a "word". So in other words your library supports only "words" not "strings" :).

Here I'll write the test, you can make it pass, lets play ping pong 🏓:

it('truncates text without spaces ', () => {
    const parameters = {
      measurements: {
        text: 5
      },
      text: 'lookitsastring'
    }

    expect(truncateString(parameters)).toEqual('looki')
  })

@albinotonnina
Copy link
Owner

albinotonnina commented Oct 21, 2020

I realized the last sentence sounded snarky. Apologies.
I guess I was trying to echo your "Is not working".
Like you said, there is an edge case that I don't cover with this library.
Therefore the library works.

To be honest I don't think that dealing with the truncation of strings without spaces would be that interesting.
It would be if the truncation would follow grammar rules, but then it would need to first deduce the language, then apply the specific rules (each language may have different rules for truncation). You see we wouldn't be talking anymore about a 1kb library.

This library, let me attempt a better definition, has the purpose to truncate sentences.
Do you have examples of other libraries that perform the truncation the way you expect it?

If you think this feature could be useful and want to contribute, feel free to open a PR and I'll review it.

@albinotonnina
Copy link
Owner

@dschinkel I updated the README. I make clear that spaces are required for the truncation to happen.
Thank you for pointing that out.

@dschinkel
Copy link
Author

dschinkel commented Oct 21, 2020

To be honest I don't think that dealing with the truncation of strings without spaces would be that interesting

I was using it to try and truncate full names of people on my site. I had to squeeze people into a grid so being able to truncate would be useful.

I would contribute but I'm really focused on getting some new stuff out for the site.

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

No branches or pull requests

2 participants