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 text-break in IE and Edge Legacy #31727

Closed
wants to merge 3 commits into from
Closed

Conversation

ihorzenich
Copy link

Fix #29319 again.
Correcting Backport #30932 to v4

word-break didn't support break-word value in IE and Edge Legacy
overflow-wrap is absent in IE/Edge Legacy (it's modern replace for word-wrap)
So we need to add back word-wrap: break-word for IE and Edge Legacy.

@XhmikosR
Copy link
Member

Thanks for the PR, but please always make sure CI passes.

@ihorzenich
Copy link
Author

@XhmikosR thanks, could you please clarify how should I proceed with Lint errors in files below?

/home/runner/work/bootstrap/bootstrap/js/src/button.js
/home/runner/work/bootstrap/bootstrap/js/src/modal.js
/home/runner/work/bootstrap/bootstrap/js/src/scrollspy.js

I didn't touch them, they're the same as current upstream v4-dev branch.

@XhmikosR
Copy link
Member

Those are not errors, they are warnings and unrelated. https://github.com/twbs/bootstrap/pull/31727/checks?check_run_id=1152664809#step:6:53

@XhmikosR
Copy link
Member

BTW I see why you got confused; GitHub changed something in regard to Actions and the Stylelint errors are not highlighted like the warnings or the parent failed commit. Either way it should be pretty trivial to fix, but for future reference better clone the repo locally so that you run the tests.

@ihorzenich
Copy link
Author

@XhmikosR thanks, I've fixed lint error and updated PR, please review.

@XhmikosR XhmikosR added this to Inbox in v4.5.3 via automation Sep 24, 2020
@XhmikosR
Copy link
Member

@twbs/css-review can I have a review just so that we are safe please?

@ihorzenich
Copy link
Author

ihorzenich commented Sep 25, 2020

@XhmikosR I've added more detailed comments how .text-break rules works.

@@ -63,8 +63,19 @@
.text-decoration-none { text-decoration: none !important; }

.text-break {
word-break: break-word !important; // IE & < Edge 18
overflow-wrap: break-word !important;
// We wan't to use `overflow-wrap: anywhere` to avoid issues with flex containers
Copy link
Member

Choose a reason for hiding this comment

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

@mdo this whole section needs grammar fixes and personally I find it too verbose

mdo added a commit that referenced this pull request Sep 30, 2020
Brings back the word-wrap property instead of overflow-wrap to avoid flex container issues. Properly backports the changes from v5 in #30932.

Closes #31727
@mdo mdo mentioned this pull request Sep 30, 2020
@mdo
Copy link
Member

mdo commented Sep 30, 2020

See #31793.

@mdo mdo closed this Sep 30, 2020
@mdo mdo removed this from Inbox in v4.5.3 Sep 30, 2020
XhmikosR pushed a commit that referenced this pull request Sep 30, 2020
Brings back the word-wrap property instead of overflow-wrap to avoid flex container issues. Properly backports the changes from v5 in #30932.

Closes #31727
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants