text and popup event height while empty

VERIFIED FIXED

Status

defect
VERIFIED FIXED
5 years ago
7 months ago

People

(Reporter: thecount, Assigned: RebeccaScheffler, Mentored)

Tracking

Details

(Whiteboard: [good first bug])

Attachments

(3 attachments)

Reporter

Description

5 years ago
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.
Reporter

Updated

5 years ago
Whiteboard: [mentor=thecount][good first bug]
Assignee: nobody → schranz.m
Status: NEW → ASSIGNED
Mentor: scott
Whiteboard: [mentor=thecount][good first bug] → [good first bug]
Assignee

Comment 1

5 years ago
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!
Reporter

Updated

5 years ago
Assignee: schranz.m → RebeccaScheffler
Reporter

Comment 2

5 years ago
The simple solution is make it so if a text event has no text, it still has height.
Assignee

Comment 3

5 years ago
Working on the text box now. I wanted to make sure this is what you were looking for.
Assignee

Comment 4

5 years ago
This patch has both fixes on it. Hope it is what you're looking for.
Reporter

Updated

5 years ago
Attachment #8498995 - Flags: review?(scott)
Reporter

Updated

5 years ago
Attachment #8501850 - Flags: review?(scott)
Reporter

Updated

5 years ago
Attachment #8498995 - Flags: review?(scott)
Reporter

Comment 5

5 years ago
I just assigned the most up to date patch to myself, and removed the old one.
Reporter

Comment 6

5 years ago
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-
Assignee

Comment 7

5 years ago
Attachment #8501958 - Flags: review?(scott)
Reporter

Comment 8

5 years ago
One final piece, and we can land this.

Could we get this patch as a github pull request?
Reporter

Comment 9

5 years ago
Staged: https://github.com/mozilla/popcorn.webmaker.org/commit/f96da8ea2eb3335e649464d1b8e077de9180ae85

On production now.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Reporter

Updated

5 years ago
Status: RESOLVED → VERIFIED
Reporter

Updated

7 months ago
Attachment #8501958 - Flags: review?(sdowne)
You need to log in before you can comment on or make changes to this bug.