The default bug view has changed. See this FAQ.

text and popup event height while empty

VERIFIED FIXED

Status

Webmaker
Popcorn Maker
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: thecount, Assigned: RebeccaScheffler, Mentored)

Tracking

Details

(Whiteboard: [good first bug])

Attachments

(3 attachments)

(Reporter)

Description

3 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

3 years ago
Whiteboard: [mentor=thecount][good first bug]
Assignee: nobody → schranz.m
Status: NEW → ASSIGNED
Mentor: scott@mozillafoundation.org
Whiteboard: [mentor=thecount][good first bug] → [good first bug]
(Assignee)

Comment 1

3 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

3 years ago
Assignee: schranz.m → RebeccaScheffler
(Reporter)

Comment 2

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

Comment 3

3 years ago
Created attachment 8498995 [details] [diff] [review]
a patch to keep the height of a pop window even if there is no text

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

Comment 4

3 years ago
Created attachment 8501850 [details] [diff] [review]
Height fix for both text and pop up event

This patch has both fixes on it. Hope it is what you're looking for.
(Reporter)

Updated

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

Updated

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

Updated

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

Comment 5

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

Comment 6

3 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

3 years ago
Created attachment 8501958 [details] [diff] [review]
made changes to height and moved semicolon
Attachment #8501958 - Flags: review?(scott)
(Reporter)

Comment 8

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

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

Comment 9

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

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

Updated

3 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.