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

Media+Text block: Remove break-all styling #15871

Merged
merged 1 commit into from
Jun 4, 2019
Merged

Conversation

mapk
Copy link
Contributor

@mapk mapk commented May 28, 2019

Fixes #15870. Removes the from the editor.scss file allowing it to break at the word.

Description

In Firefox, the content of this block would break in the middle of a word. It shouldn't.

How has this been tested?

Tested locally.

Screenshots

mediatext-fix

Types of changes

Minor CSS change.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@mapk mapk self-assigned this May 28, 2019
@mapk mapk added the [Block] Media & Text Affects the Media & Text Block label May 28, 2019
@mapk
Copy link
Contributor Author

mapk commented May 28, 2019

To test this:

1. Text this in the Firefox browser.
2. Add a Media+Text block to your document.
3. Begin typing in the content area of this block until the text drops to a second line.
4. Observe whether or not the text broke in the middle of a word, or dropped the word down to the next line entirely.

The latter is the correct solution. It should drop the entire word to the second line and not break word in the middle.

@gziolo
Copy link
Member

gziolo commented May 29, 2019

Noting that it was added in #14951 by @marekhrabe to fix some other issues. So it should be retested in that context as well.

@gziolo gziolo requested a review from marekhrabe May 29, 2019 06:38
@mapk
Copy link
Contributor Author

mapk commented May 29, 2019

Thanks @gziolo! I knew it was there for a reason but forgot.

@marekhrabe any thoughts on this? After reviewing it from a design perspective, I'd like to just break at whole words. It feels better and visually makes more sense. When breaking at letters, the breaks look very odd, especially when there's plenty of space in the content area of the block.

I'm willing to take the tradeoff of words expanding outside the block border in edge cases (really narrow text areas) over words breaking in the middle in more common width scenarios.

@marekhrabe
Copy link
Contributor

The problem was with the drag & drop resizing action: In some cases, words could overflow outside of the box and thus changing the width of the element. That caused a janking effect as the media width is defined as a percentage but the base size was also changing constantly. It was definitely an edge case and seemed to be browser specific so for greater good, I think we should probably give this issue a priority as it applies everywhere.

@mapk mapk added the Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code label May 29, 2019
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

This works as expected and in accordance with @marekhrabe comment:

That caused a janking effect as the media width is defined as a percentage but the base size was also changing constantly. It was definitely an edge case and seemed to be browser specific so for greater good, I think we should probably give this issue a priority as it applies everywhere.

I think we should merge this change 👍

@mapk mapk merged commit 672599a into master Jun 4, 2019
@mapk
Copy link
Contributor Author

mapk commented Jun 4, 2019

Thanks @jorgefilipecosta!

@mapk mapk deleted the update/media-text-break-word branch June 4, 2019 22:59
@youknowriad youknowriad added this to the Gutenberg 5.9 milestone Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Media & Text Affects the Media & Text Block Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Media+Text block: The text breaks in the middle of words
5 participants