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

wordBreak Option, cleaned up logic on what function to use #28

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

AndyDyer
Copy link

…wordBreak option and cleaned up fallBackFunc logic

@AndyDyer AndyDyer changed the title cleaned up logic on what function to use browser depending, added in … wordBreak Option, cleaned up logic on what function to use Feb 27, 2019
@AndyDyer AndyDyer changed the base branch from master to develop February 27, 2019 17:49
@AndyDyer AndyDyer changed the base branch from develop to master February 27, 2019 17:49
@alejandroiglesias
Copy link

Is there any intention of merging either this or #27?

@AndyDyer
Copy link
Author

@alejandroiglesias I'd like to but I don't have write access

@AndyDyer
Copy link
Author

When I made this develop was out of date so I didn't know what the protocol was

@AndyDyer AndyDyer changed the base branch from master to develop March 23, 2019 20:25
@AndyDyer AndyDyer changed the base branch from develop to master March 23, 2019 20:25
@Frondor
Copy link
Owner

Frondor commented Apr 1, 2019

I'm fine with this. Will test it locally tomorrow.

Can you add it to the readme, please?

@franciscolourenco
Copy link

Does it make sense to have wordBreak: 'break-all' on by default when it is not the browser default? It prevents proper break of phrases which would fit in. What does it accomplish?

@Frondor
Copy link
Owner

Frondor commented Apr 1, 2019

Because the ellipsis approach works better with break-all. The symbol is supposed to be at the very limit of the container, not floating to the last word that fits in.
https://stackoverflow.com/questions/1795109/what-is-the-difference-between-word-break-break-all-versus-word-wrap-break

@AndyDyer
Copy link
Author

AndyDyer commented Apr 1, 2019

done @Frondor

@franciscolourenco
Copy link

franciscolourenco commented Apr 2, 2019

@Frondor Can you provide an example of what you mean? When I set word-break: break-word the ellipsis seems to work exactly the same way. It still breaks works and takes advantage of all the space in the second line:

break-all
Screen Shot 2019-04-02 at 11 19 08

break-word
Screen Shot 2019-04-02 at 11 18 48

@AndyDyer
Copy link
Author

AndyDyer commented Apr 2, 2019

Not that I disagree @franciscolourenco but making this change to default behavior is a breaking change, and this PR would allow people to choose their path.

@franciscolourenco
Copy link

@AndyDyer That is great. You could classify this a bug, since it is not documented or expected that this plugin changes the way the words break. In that case making the default break-word would be fixing it. Otherwise the major version can be incremented.

@AndyDyer
Copy link
Author

AndyDyer commented Apr 3, 2019

Up to @Frondor

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
Frondor and others added 3 commits April 3, 2019 09:22
Co-Authored-By: AndyDyer <[email protected]>
Co-Authored-By: AndyDyer <[email protected]>
Co-Authored-By: AndyDyer <[email protected]>
@AndyDyer
Copy link
Author

@Frondor any more changes?

@phouri
Copy link

phouri commented Jun 6, 2019

+1 on this please, we need it.

@AndyDyer
Copy link
Author

AndyDyer commented Jun 7, 2019

@Frondor bump

@do-adams
Copy link

This is great, I could really use this as I was wondering why my word-break value wasn't being honored and it turns out that this directive hardcodes the break-all value as an inline style. I would love to be able to customize this.

@Frondor any blockers on merging this in?

@roennow
Copy link

roennow commented Nov 1, 2019

@Frondor anything holding this up? If you need help just say it

@Devin0231577
Copy link

Bump

@gotson
Copy link

gotson commented Nov 20, 2019

Hello, i am also interested by this one, is there any chance this PR could go through? :)

@mezereon-co
Copy link

+1 on getting this PR approved and released

@patryk-tech
Copy link

Nice. Note that the default word-break: break-word; is deprecated according to the MDN.

[break-word has] the same effect as word-break: normal and overflow-wrap: anywhere, regardless of the actual value of the overflow-wrap property.

Maybe using word-break: normal; as the default and adding an overflow-wrap would be preferable?

@paulkarpenko
Copy link

@Frondor Can this be merged please? This is a crucial feature to present readable text.

@kirillunlimited
Copy link

@Frondor When this fix will be merged? The only option to use this lib now is to fork it and fix the issue locally 😔

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