Closed Bug 1019657 Opened 6 years ago Closed 6 years ago

text and popup event height while empty

Categories

(Webmaker Graveyard :: Popcorn Maker, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: thecount, Assigned: RebeccaScheffler, Mentored)

Details

(Whiteboard: [good first bug])

Attachments

(3 files)

If a popup or text event has no text, it also has no height.

We could keep the height the same with or without height. Right now you can get this if you add a single space to the text, but I see no value in squashing it. It just makes it hard to click.
Whiteboard: [mentor=thecount][good first bug]
Assignee: nobody → schranz.m
Status: NEW → ASSIGNED
Mentor: scott
Whiteboard: [mentor=thecount][good first bug] → [good first bug]
Hello Scott-
 I see that Matthew Shranz is currently assigned to it, but this looks like a good first bug and I am really excited to take a crack at it. My current understanding is that I need to add some code to make the event and text popup have height even if there is no text in the window. This would by my first bug so I may need some guidance but I have pulled the code from https://github.com/mozilla/popcorn.webmaker.org.git and think I'm on the right track. Thank you for your time!
Assignee: schranz.m → RebeccaScheffler
The simple solution is make it so if a text event has no text, it still has height.
Working on the text box now. I wanted to make sure this is what you were looking for.
This patch has both fixes on it. Hope it is what you're looking for.
Attachment #8498995 - Flags: review?(scott)
Attachment #8501850 - Flags: review?(scott)
Attachment #8498995 - Flags: review?(scott)
I just assigned the most up to date patch to myself, and removed the old one.
Comment on attachment 8501850 [details] [diff] [review]
Height fix for both text and pop up event

Just change the height for popcorn.popup.css to be min-height too, to be like the height change in popcorn.text.css that you added.

That way it can still support multi line popup's like the one used in this test project I made: https://popcorn.webmaker.org/en-US/editor/108974/remix

And fix up the stray semi-colon in popcorn.text.css, and this should then be good to go :)
Attachment #8501850 - Flags: review?(scott) → review-
Attachment #8501958 - Flags: review?(scott)
One final piece, and we can land this.

Could we get this patch as a github pull request?
Staged: https://github.com/mozilla/popcorn.webmaker.org/commit/f96da8ea2eb3335e649464d1b8e077de9180ae85

On production now.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Attachment #8501958 - Flags: review?(sdowne)
You need to log in before you can comment on or make changes to this bug.