search

mozilla / webmaker-core

React-based core for Webmaker shared across all platforms
https://foundation.mozilla.org/en/artifacts/webmaker/
Mozilla Public License 2.0
65 stars 39 forks source link

Long strings of text containers behave erratically in page edit mode #961

Closed alanmoo closed 8 years ago

alanmoo commented 8 years ago
  1. Create a long text element (30 characters or so)
  2. Back on the page, while trying to position/scale it you will see that the bounding box doesn't surround the whole piece of text because of a max-width of the element likely not taking into account the scale of the element.

This should...not be weird

Keep in mind when fixing this that we don't want to affect the way text elements render on existing projects

xmatthewx commented 8 years ago

Alas, seems this was introduced by https://github.com/mozilla/webmaker-core/pull/924/files. If we can't find an easy fix, let's remove this and ignore my request to retain trailing and leading spaces by users.

They way we handle text wrap of a scalable element has always been a bit odd (linking fontsize bounding box), but now's not the time to reinvent how we handle text elements.

xmatthewx commented 8 years ago

Since @alanmoo is out, let's remove this patch for now and confirm that it fixes the bug.

gvn commented 8 years ago

@alanmoo now that you're back, wanna take a look see?

alanmoo commented 8 years ago

Sure thing!

alanmoo commented 8 years ago

I believe I can fix this by removing the max-width of elements in the editor CSS. However that only fixes the visual bug, there's still a question of expected UI behavior and text wrapping. Currently (and with this possible fix), elements simply won't wrap, and there's no way to add a manual line break because the Android keyboard doesn't offer it (presumably because it's an input element). Is this ok? I'm inclined to believe it is, as enabling line breaks encourages longer pieces of text, but I'm also not sure what users want here. Thoughts, @xmatthewx?

xmatthewx commented 8 years ago

I wasn't a part of the discussion when this was originally implemented. I know there was a bit of debate on how to handle this.

At this stage, we have to maintain the rendering of existing text elements, which have a forced line-wrap based on initial font-size and scale. If we try and remove one errant stitch, we'll likely unravel our sweater.

We don't have time to implement what I think is the ideal solution... two text elements: Headers (font scales with element, no-wrap, like Alan describes), Paragraphs (bounding box resizes and text reflows, font-size set with UI slider). Maybe in a future release.

For now, let's just look at that patch and see how much of it we need to roll back to fix this.

Make sense?