Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

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

Closed
alanmoo opened this issue Dec 23, 2015 · 6 comments
Closed
Assignees
Labels
Milestone

Comments

@alanmoo
Copy link
Contributor

alanmoo commented Dec 23, 2015

  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

@alanmoo alanmoo added the bug label Dec 23, 2015
@xmatthewx xmatthewx added this to the 1.6.0 milestone Dec 26, 2015
@xmatthewx
Copy link

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
Copy link

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

@xmatthewx xmatthewx assigned gvn and unassigned alanmoo Jan 4, 2016
@gvn gvn assigned alanmoo and unassigned gvn Jan 11, 2016
@gvn
Copy link
Contributor

gvn commented Jan 11, 2016

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

@alanmoo
Copy link
Contributor Author

alanmoo commented Jan 11, 2016

Sure thing!

@alanmoo
Copy link
Contributor Author

alanmoo commented Jan 11, 2016

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
Copy link

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants