947300 - Text plugin's link target selector should not be visible unless a URL has been entered

Status: RESOLVED WONTFIX

(Webmaker Graveyard :: Popcorn Maker, defect)

Description

[u482081]

Currently, the link target selector is available through UI even when no link address has been entered. It should be hidden until a URL has been entered.

Comment 1

[u482081]

Attachment: https://github.com/mozilla/popcorn.webmaker.org/pull/372

You were right Scott, that was trickier than I thought it would be, but I think I got a handle on the current design. Any thoughts?

Comment 2

[Scott [:thecount] Downe]

Just a nit, but I noticed there is a comma in "Open Link In:" but none of the other dialogs have a comma. Can probably remove it.

Comment 3

[Scott [:thecount] Downe]

Comment on attachment 8344228 [details] [diff] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/372

Some pull request comments.

Comment 4

[u482081]

Comment on attachment 8344228 [details] [diff] [review]
https://github.com/mozilla/popcorn.webmaker.org/pull/372

Everything should be fixed now

Comment 5

[Scott [:thecount] Downe]

I think at this point, this ticket has merged into bug 947303. Can probably close this one now?

Comment 6

[u482081]

Originally, I was including these text editor changes in bug 947303 because they had shared functionality in trackevent-editor.js. Not sure why I've still been doing it, but I'd say this is still a distinct issue. I've updated the pull request for this bug with another commit incorporating your feedback on bug 947303, if you'd like to take a look.

Comment 7

[Scott [:thecount] Downe]

I think this served its purpose, but, now that bug 947303 is almost done, and does what we need, I still don't think we have any more use of this ticket.

Am I right to say that once bug 947303 lands, it's going to bring in the changes this ticket has?

Putting the changes from this ticket into bug 947303 was probably the right thing to do.

Comment 8

[u482081]

Yep, you're right. Should be fine to close, if that's what you want to do. Bug 947303 should be completed, BTW. I addressed Matt's nits and changed the default selector value.