New widget: Popup #3218

Closed
toddparker opened this Issue Dec 5, 2011 · 42 comments

Projects

None yet
@toddparker

This is a new widget to open any content in a simple overlay. Based on this plugin: http://babulina.go-nix.ca/nix/jquery-mobile-popup/docs/pages/popup/index.html

To turn any div into a popup, add the data-role="popup" attribute to the div. If you would like the popup to appear when the user clicks on a link, create a link such that its href attribute is set to the id of the popup, and the link has the attribute data-rel="popup".

<a href="#popupBasic" data-rel="popup">Tooltip</a>

<div data-role="popup" id="popupBasic">
    This is a completely basic popup, no options set.
</div>

This will result in a popup which is essentially a semi-transparent themable overlay with a popup centered over it that can contain any content, widget or form element, including dialog markup. Note that we shouldn't set any padding on the popup so widgets will fit cleanly inside. The overlay and popup should both be themeable.

Also consider leveraging this dialogs and custom selects.

@Wilto Wilto was assigned Dec 5, 2011
@scottjehl

Hey Todd and Mat.

I added some preliminary support for the back button today. It's done externally from this plugin, and as I was toying with it the thought occurred to me that we might do well if core fired off some "back" and "forward" events when it is pretty sure that one of those respective actions has occurred. Then we could bind to those instead of reinventing things each time...

Anyway, the commit is here:
d2c7bbe

Things seem to work in Chrome, but it'd be nice to hear how it tests out in the lab before pushing ahead. I'm sure some history edge cases are waiting to be found.
Thanks!

@iBobik
iBobik commented Dec 10, 2011

Why don't do it look like native iPad's floating window? Screenshot: http://dl.dropbox.com/u/3306439/Photo%2010.12.11%2011%2025%2013.png

@toddparker

Because we're not trying to emulate the look and feel if a particular platform and want more of a general use popup. Adding the little carat increases the coding complexity quite a bit for little effort. Other than that element, you'd be able to achieve a similar design to the iOS popover with just a bit of CSS.

@gabrielschulhof
Collaborator

@toddparker and @iBobik :

We already have code for adding the triangle to the popup [1], but I haven't contributed it yet, because I figured it was web-ui-fw-specific. It also requires a second widget, "triangle". However, if you think it may be worth contributing, then I'm good for making a pull request for it.

[1] http://web-ui-fw.github.com/index.html#popupwindow-demo

@iBobik
iBobik commented Jan 11, 2012

Its good if it is part of jQuery Mobile. Thank you.

@gabrielschulhof
Collaborator

@iBobik it's not part of jqm, nor did I intend it to become a part of jqm. It is currently available only in web-ui-fw. I'm only mentioning it in case the jqm developers should decide that's it's an important enough addition to popup for me to submit to jqm.

@iBobik
iBobik commented Jan 11, 2012

Aha. I like this widget. Let's wait for jQM devs feedback :-)

@toddparker

@gabrielschulhof - That is a cool demo. Curious how much overhead that adds for collision detection vs. just popping up over the thing like the simple popup. Do you have a sense? In order for this to look polished, we'd need to set the arrow color to match the gradient color because it doesn't line up so that will add some complexity unless popups just use the flat bg color.

In any case, the arrow code could be built as an extension to the standard popup which is how we added the listview filter. Our goal is to keep the API and feature set as minimal as possible in the base widget and use optional extensions to layer on more features. Once popup is done, we should look at adding this.

I didn't see the transition type support in the version we pulled in. If that was lightweight to add, we might want to add that to the core widget. Let me know, maybe we can have you do a pull request for that in the popup branch if there isn't a lot of code for it. I do wonder how our new transition scripting will work with this...that will require some investigation once that lands this week.

@gabrielschulhof
Collaborator

@toddparker

@gabrielschulhof - That is a cool demo. Curious how much overhead that adds for collision detection vs. just popping up over the thing like the simple popup. Do you have a sense?

It's actually pretty simple. The existing code that decides where to put the popup is repeated once for a location above the desired coordinates, and once for a location below. Then, the coordinates which are closest to those desired are chosen for the popup.

In order for this to look polished, we'd need to set the arrow color to match the gradient color because it doesn't line up so that will add some complexity unless popups just use the flat bg color.

I'm not sure what we can do here, because the arrow is currently nothing but the bottom (resp. top) border of a 0-sized div whose left and right borders are transparent and whose border width is actually the height of the triangle. Here's where I got the idea from. AFAIK borders cannot be gradients. OTOH, I recall one of the demos you added to the documentation was that of an image with a (X) in the top left corner sticking out of the body of the popup. We might do the same thing to get the arrow looking right.

In any case, the arrow code could be built as an extension to the standard popup which is how we added the listview filter. Our goal is to keep the API and feature set as minimal as possible in the base widget and use optional extensions to layer on more features. Once popup is done, we should look at adding this.

Yeah, let's have popup in first, and go from there.

I didn't see the transition type support in the version we pulled in.

It should be there. Look for _set_transition: function(value, unconditional).

If that was lightweight to add, we might want to add that to the core widget. Let me know, maybe we can have you do a pull request for that in the popup branch if there isn't a lot of code for it.

I can make a pull request with examples of setting different transitions added to the documentation. There's already such a demo on the web-ui-fw demo page (click the "Choose transition" button).

I do wonder how our new transition scripting will work with this...that will require some investigation once that lands this week.

We should rebase popup onto this branch and have a look.

@imaginethepoet

I'd love to use this right now, and thought I had it working but when I grabbed 1.01 I'm not sure if it's in that branch. I remember Todd mentioning on a post somewhere to de-couple this so it could be used as a widget for the time being. It has a lot of potential.

Good work.

@imaginethepoet

So after extensive hours of playing with this. I am unable to get this to work (explanation).

  1. It appears that it will only initialize and work in the first page 1 time. If you have a multi-part page and go from page1 to page 2 it will not work again.

I'm thinking that something needs to be re-initialized in each pageload - to make sure the popup is loaded into the dom.
I tried the simplest of multi-page examples and on foo 2 you will see that the popup does not load.

gist.github.com/1723838.git

I have a simple 2 part multipage example to show this - but git won't let me post the code for some reason.

@gabrielschulhof
Collaborator

@imaginethepoet I can't see anything when I try to load gist.github.com/1723838.git ... can you post a URL with an example of the breakage?

@imaginethepoet

Sorry new to this git thing - I posted the raw html on jsfiddle

http://jsfiddle.net/9C4EW/

@gabrielschulhof
Collaborator

@imaginethepoet My first reaction is that the problem is that you have two different popups named popupTesterThemed on two different pages. Two different elements should never have the same ID.

@imaginethepoet

I understand that, but then doing a menu that always has the same options isn't an option unless the ID's are changed to classes. Which I tried as well and that failed.

Should the popup not work this way in a multi-part page? I tried it with just one "menu" and then called it via links in page 1 and page 2. Still no luck. The first one would fire 1 time on initial page init.

I think something needs to be added on each multi-part page to reinitialize the popup so it's available to the entire document. It looks like right now it only as available.

@toddparker

If you wanted to re-use a popup, you could just append a single copy with the same ID dynamically when needed but IDs always have to be unique and that is how we reference the popup. A class doesn't work because the link's href (#foo) needs to point to a unique popup element to show, that's why an ID is needed.

@MHdesign
MHdesign commented Feb 3, 2012

I've been trying to use this simple popup code in my mobile app using Dreamweaver 5.5 and I still can't get it to work. I've tried using the code in my data-role="page" and linked to the "jquery.mobile.popup.js" and the "jquery.mobile.popup.css" in addition to the linked updated libraries for jQuery and jQuery mobile and it won't work. What can I be doing wrong? Going out of my mind.

@gabrielschulhof
Collaborator

@imaginethepoet You've actually helped me identify a bug in the popup widget. Thanks! It seems that (this.element.closest(".ui-page") || $("body")) will no longer result in $("body") if no closest page is found. I've fixed this in PR#3540.

In the meantime, have a look at this. It implements a popup shared among both windows. You have to do it by hand, and not with <a href="#thePopup">, because the popup window is not on the same page as the button that activates it.

I've run into a bug when I tried to change the line

thePopup.popup();

to

thePopup.trigger("create")

to not only create the popup itself, but also the listview inside the popup. The bug seems to be in the listview. It references the page it resides in, and, of course, this particular listview does not reside inside any page.

@gabrielschulhof
Collaborator

@MHdesign I know the feeling ... can you post a simple example of what you've been trying to do?

@imaginethepoet

Nice, glad I at least help you identify a bug before release in 1.2 :)

@huan086
huan086 commented Feb 18, 2012

== Bug ==
When popup opens, the url gets incorrectly set to first.html#&ui-state=dialog instead of second.html#&ui-state=dialog.
Clicking the browser's back button does not close the popup as a result.

== Steps to reproduce ==

Have ajaxEnabled enabled
Create the first page first.html with link to second.html
Create the 2nd page second.html that uses popup
Navigate to first.html
Click on link to second.html
Click on the button to show popup
Observe that it shows first.html#&ui-state=dialog instead of second.html#&ui-state=dialog
Click the back button
Observe that it failed to close the popup

== Solution ==
In the open method, change

$.mobile.path.set("&ui-state=dialog");

to

$.mobile.path.set((self.element.closest(":jqmData(role='page')").jqmData("url") || "") + "&ui-state=dialog");

Note: this solution does not solve the problem in Android browser, but is effective for iPhone and Chrome.

@Hexodus
Hexodus commented Apr 7, 2012

Very useful plugin - thank you for sharing!
I just found minor issues with calling the popup directly like this:

$("#popupBasic").popup('open');

-> This results in a darkened screen but with no popup.

$("#popupBasic").popup('open', x_pos, y_pos);
-> This one works fine. I guess there are no default coordinates defined for fallback situations.

It's also necessary to set data-theme or the popup will appear with transparent background.

@gabrielschulhof gabrielschulhof added a commit that referenced this issue Apr 25, 2012
@gabrielschulhof gabrielschulhof [popup] Ensure at opening time that an overlay theme has been set - a…
…ddresses Hexodus' comment on issue #3218
002c498
@jeffmhastings

Can't wait for this plugin to make it into a release. It is badly needed.

I realize we can use the open method to position the popup on the screen, but how about adding some options for positioning in the markup? Then this widget could be used as an overlay instead of just a tooltip. I like being able to use the "data-rel" attribute on the link, rather than binding an event handler and opening the popup manually.

@toddparker

Hi @jeffmhastings - We're looking forward to getting this plugin out too. So are you asking for positioning options for the popup itself? Currently, the popup is centered over the thing you tap as long as it fits inside the viewport, otherwise we center it up.
http://jquerymobile.com/branches/popup-widget/docs/pages/popup/index.html
(still a few bugs)

If we wanted to have more granular control, we'd probably have to look at pulling in the position plugin from jQuery UI. Or building this in such a way that you could optionally add that for more flexibility. For the intiial release, we're trying to keep this as simple as possible so this seems like a future enhancement,

@huan086
huan086 commented Apr 28, 2012

@jeffmhastings: The latest release seems to be working perfectly with jQuery Mobile 1.1.0 if you DISABLE ajax navigation. You can use data-rel on a link. See http://jquerymobile.com/branches/popup-widget/docs/pages/popup/index.html

Some changes in jQuery Mobile 1.1.0 seems to have caused the popup to fail when ajax navigation is enabled. Specifically, after the popup opens, using the either of the following methods causes the page to navigate to the previous page

  1. clicking on the screen overlay

  2. browser back button

  3. clicking a a button with data-rel="back"

  4. used to work in jQuery Mobile 1.0.1...

Edit:
Looks like it is not ajax navigation causing the problem, but pushstate. The popup works perfectly with the following config.
$(document).bind("mobileinit", function () {
$.mobile.pushStateEnabled = false;
});

@huan086
huan086 commented Apr 28, 2012

== Bug ==
In a page without scrollbars, opening the popup causes vertical scrollbars to appear.

== Cause ==
In jquery.mobile.theme-1.1.0.css, .ui-body-a has the style "border: 1px solid #333". Since the popup-screen has the class .ui-body-a, it is bigger than it needs to be by 2px, causing the scrollbar.

== Solution ==
Add border: 0 to the .ui-popup-screen class

.ui-popup-screen {
border: 0;
}

@adammessinger

This widget was exactly what I needed to provide a touch-friendly equivalent to tooltip help on forms for my current project. Thanks!

I noticed that popups with more than a very small amount of text end up extending beyond the vertical borders of the screen on small-screen devices. The following CSS corrects the problem:

.ui-popup-container {
    max-width: 90%;
}

I suspect that the behavior @huan086 describes above -- closing the popup with the back button -- is a feature not a bug. It makes a lot of sense, given that Android users are accustomed to using the platform's built-in back button to dismiss dialogs and pop-up controls (like select menus).

That said, it might be nice to have a data-attribute option for a close button on these pop-ups. This would give less savvy users a more immediately obvious way to dismiss them. Two of my current project's four (non-me) testers immediately asked upon opening the popup help "okay how do I get rid of this" and "where's the close button?"

Other minor issues:

  • The background-dimming overlay doesn't apply to fixed toolbars, at least not to the true fixed toolbars of jQM 1.1.
  • Some built-in padding would help improve the default visuals of the pop-ups by giving the content some breathing room. I realize this might not work for every kind of pop-up content, however, so perhaps it's best left to the developer's custom styles.
  • The widget adds the ui-corners-all class to the outer .ui-popup-container div and data-corners="true" to the inner data-role="popup" div. However, the inner div never gets the corresponding class and the corners that go with it. Since the inner div has the higher z-index, the net result is square corners.
  • If you have dialog transitions disabled globally and don't declare one for your pop-ups, there's an awkward lag to the appearance of the background overlay on mobile devices. It fades in and out smoothly on a desktop machines, but on my iPod Touch and Galaxy Nexus it appears all at once after a short wait. Should probably check the global dialog setting and show/hide the overlay instantly if dialog transitions are off.
@toddparker

Yes, the back button support is an important and expected behavior on a lot of platforms as @adammessinger suggests. I agree that adding an optional way to add a close button like on a dialog would be a good feature to add, especially since you had so much feedback that it's confusing. Thanks for reporting these issues, keep them coming but I'd suggest logging separate issues going forward instead of piling them into one issue.

@adammessinger

Would it be helpful if I created new issues for the stuff covered in my last comment, or is that okay to leave here?

@toddparker

@gabrielschulhof - what do you prefer? Separate issues for the current ones here or can you work against this?

@gabrielschulhof
Collaborator

Separate issues please, thank you :)

@adammessinger

I've added separate issues for the problems reported in my earlier comment, and referenced this discussion to avoid any confusion of the popup widget and the dialogs that more people are probably familiar with.

If the reference additions to the comments here are a nuisance, please let me know and I'll knock it off. :)

@adammessinger

To what extent does this widget depend on changes that aren't in 1.1.0? Just tried to use the latest version of the popup widget with the 1.1.0 release, and I'm getting the following error:

$.mobile._maybeDegradeTransition is not a function
https://raw.github.com/jquery/jquery-mobile/popup-widget/js/jquery.mobile.popup.js
Line 136
@gabrielschulhof
Collaborator

Yeah, I guess I added that ...

@patik
patik commented May 20, 2012

Is there a way to find out which link invoked the popup? For example, suppose you have these two links pointing to the same [data-role="popup"]:

<a href="#myPopup" data-rel="popup" class="popup-link">Link 1</a>
<a href="#myPopup" data-rel="popup" class="popup-link">Link 2</a>

I have tried using this but it never runs:

$('body').on('tap', '.popup-link', function() { console.log('hello'); });

And the opened method for the popup only references the popup itself.

@adammessinger

@cpatik Have you tried using the "click" or "vclick" events instead of "tap"?

@MauriceG

Hi @cpatik
Here is a fiddle http://jsfiddle.net/MauriceG/XuZ6C/show/
(It seems, popups does not work correct in jsfiddle edit mode. After the first popup is shown and hidden, no popup comes up anymore.)

@patik
patik commented May 21, 2012

Thanks, @MauriceG and @adammessinger, that does seem to do the trick. There must be something else in my code canceling the event. As a work-around for now, I can use vmousedown on the links which is helpful because my goal is to prepare the popup's contents before it appears, based on which link was clicked

@Sathis
Sathis commented Jul 4, 2012

popup widget(http://jquerymobile.com/branches/popup-widget/docs/pages/popup/index.html) is not working in Android 2.3.5.

@jaspermdegroot
Member

@Sathis

That branch is not updated since it got merged into master two months ago. Can you try http://jquerymobile.com/test/docs/pages/popup/index.html instead?

@jaspermdegroot
Member

I am closing this ticket now the popup widget has landed in master. You can open a new ticket to report an issue with the popup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment