Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Popup] Review Edge Detection #2526

Closed
tazator opened this issue Jul 6, 2015 · 19 comments
Closed

[Popup] Review Edge Detection #2526

tazator opened this issue Jul 6, 2015 · 19 comments

Comments

@tazator
Copy link

tazator commented Jul 6, 2015

Hi everyone!
I have a problems with popup on Chrome 43.0.2357.130.
The popup with click event, it's ok.

//WORKS
$('.ui.icon.button').popup({on:'click'});

But with hover event (by default), the popup don't show.

// WILL NOT WORKS
$('.ui.icon.button').popup();

Example of code

<div class="ui icon button" data-content="Add users to your feed">
  <i class="add icon"></i>
</div>

I have the same problem with my website or with semantic-ui website
On Firefox no problems (click and hover)

@jlukic
Copy link
Member

jlukic commented Jul 6, 2015

Please fork the jsfiddle in readme to create a test case.

@BigGrecian
Copy link

tazator : http://codepen.io/Biggrecian/pen/vOdwLd is this what you are experiencing in a menu? Number 1 works but numbers 2 and 3 don't ?

If so try the code :
$('.example .menu .browse') .popup({ inline : true, hoverable: true, position : 'bottom left', delay: { show: 300, hide: 800 } }) ;

@tazator
Copy link
Author

tazator commented Jul 7, 2015

jlukic : I'm sorry, here is http://jsfiddle.net/Le9ys6zk/7/
On Chrome btn1 doesn't work, and btn2 works
On Firefox btn1 and btn2 works

I have a same problem on http://semantic-ui.com/modules/popup.html

BigGrecian : On my Google Chrome (v 43.0.2357.130) numbers 1, 2 and 3 doesn't work
Just works with code :

$('.status1.item').popup({on:'click'});

@basicserge
Copy link

Completely the same problem as @tazator mentioned. (Chrome 43.0.2357.132, win 8 64bit)

@jlukic jlukic added this to the 2.0.4 milestone Jul 13, 2015
@jlukic jlukic changed the title Problems with popup on Chrome [Popup] Review Edge Detection Jul 13, 2015
@jlukic
Copy link
Member

jlukic commented Jul 13, 2015

@jlukic
Copy link
Member

jlukic commented Jul 13, 2015

It appears with elements that are "just next to" the edge of the page it returns an offset slightly below 0, due to rounding issues.

I've added a new setting jitter which is the amount of pixels a popup is allowed to exceed the boundary of its context. Default setting is 2 pixels, although in most cases this will be < 1 pixel

@jlukic
Copy link
Member

jlukic commented Jul 13, 2015

With regards to this example,
http://codepen.io/krobinson/pen/gpKMPQ

The first button is not showing because the popup is set to 100% page width but is pointing to an element in the middle of the page. This would mean most of the popup would have to display off screen.

jlukic added a commit that referenced this issue Jul 13, 2015
… offsetContext (i.e. thing with position: relative)
@jlukic
Copy link
Member

jlukic commented Jul 13, 2015

@ksr583 I've figured out what's going on for your issue.

Popups can't use width: 100%; for positioning when inline because the popup will always exceed its parents containers width. The way around this was a setting setFluidWidth and internally setting the css width in JS to offsetContainer.width on show.

However in your example, although the element is inside a container its offsetContext is body since there is no position: relative; on container (or transform context but lets not go there).

The solution is in the case of fluid popup to also check .parent() element width, regardless of whether it is offsetContext and use that for fluid width. See 5d113c0 above

jlukic added a commit that referenced this issue Jul 13, 2015
jlukic added a commit that referenced this issue Jul 13, 2015
@jlukic
Copy link
Member

jlukic commented Jul 13, 2015

@ksr583 Working example JSfiddle from next patch
http://jsfiddle.net/hg20jadx/

@jlukic
Copy link
Member

jlukic commented Jul 14, 2015

Let me know if anyone experiences any issues testing against next. This will land with next patch

@jlukic jlukic closed this as completed Jul 14, 2015
@tazator
Copy link
Author

tazator commented Jul 20, 2015

I'm so sorry but the issues is again with chrome (Version 43.0.2357.134 m) on windows 8.1 64bits. The issues is just with cursor mouse. But with tactile function no problem, it's ok.

With chrome (Version 43.0.2357.134 m) on windows 7 64bits no problem, it's ok.

@jlukic
Copy link
Member

jlukic commented Jul 20, 2015

Test case please

@tazator
Copy link
Author

tazator commented Jul 23, 2015

http://jsfiddle.net/626b86v9/5/

With chrome (Version 44.0.2403.89 m)
On Windows 8.1 64bits, cursor hover event and click doesn't work, but tactile function work.
On Windows 7 64bits all work

@jlukic
Copy link
Member

jlukic commented Jul 23, 2015

I'm not able to replicate on win7 (which you says works)

Are you using a tablet computer? Something with both touch and mouse?

@tazator
Copy link
Author

tazator commented Jul 23, 2015

Yes my computer has a touchscreen (it's laptop)
@basicserge has the same problem on Win 8

@jlukic
Copy link
Member

jlukic commented Jul 23, 2015

Yeah, I've dealt with this issue.. years ago. #61

hasTouch is being used to determine events, but in cases where both are available it fails to add mouse events (which is to prevent two events firing from simulated events)

It's a pretty frustrating bug to deal with but I'll see what I can do

@tazator
Copy link
Author

tazator commented Jul 23, 2015

But with semantic-ui 1.x on same computer, the hover event of mouse work (if that help)

@jlukic
Copy link
Member

jlukic commented Jul 23, 2015

Yes it does help. I can see the difference

https://github.com/Semantic-Org/Semantic-UI/blob/master/src/definitions/modules/popup.js#L887 this probably should be if not else if

@jlukic
Copy link
Member

jlukic commented Jul 23, 2015

Please continue discussion in #2715

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

No branches or pull requests

4 participants