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

(Bug) Iphone 5 - Safary - Popover && Tooltip - Tooltip does not close when focus is lost #345

Closed
cvaize opened this issue Jan 8, 2020 · 42 comments
Assignees
Labels
enhancement potential improvement

Comments

@cvaize
Copy link
Contributor

cvaize commented Jan 8, 2020

Tooltip does not close when focus is lost. Compared with the original "bootstrap 4", everything works correctly in it, so in "bootstrap.native" I consider this a bug.
https://drive.google.com/file/d/1E6spqe92n79j1nXqG4TCQk7DWiKiroOd/view?usp=sharing

@cvaize
Copy link
Contributor Author

cvaize commented Jan 8, 2020

@thednp Watch the video

@thednp
Copy link
Owner

thednp commented Jan 8, 2020

Check latest master, I believe queryElement is responsible for this one too.

@thednp
Copy link
Owner

thednp commented Jan 8, 2020

One more suggestion: please try the previous version of our polyfill.

@thednp
Copy link
Owner

thednp commented Jan 8, 2020

This is strange indeed, the last build won't work on my Android phone as well, but I have another build working with currentTouches.

@thednp
Copy link
Owner

thednp commented Jan 8, 2020

My bad, it's changedTouches and not currentTouches.

@thednp thednp added the bug critical issue label Jan 8, 2020
@thednp thednp closed this as completed Jan 8, 2020
@thednp thednp reopened this Jan 8, 2020
@thednp
Copy link
Owner

thednp commented Jan 8, 2020

I solved the carousel swipe bug, but I don't know about Popover, Tooltip, please confirm with latest master?

@cvaize
Copy link
Contributor Author

cvaize commented Jan 9, 2020

The problem remains, Popover and Tooltip do not close when you lose focus

@thednp
Copy link
Owner

thednp commented Jan 9, 2020

Please tell does the demo on gh-pages do the same on iPhone? There could be something causing this.

@cvaize
Copy link
Contributor Author

cvaize commented Jan 9, 2020

@thednp
Copy link
Owner

thednp commented Jan 10, 2020

@cvaize it seems we don't have any filters/checks on the handlers or any handlers targeted at mobile devices, I believe we need to use touchstart on document while trigger option is set to hover, I will have a look into it as soon as I can.

Meanwhile, since I don't have any iPhone/iMac or any Apple, I need to know what events trigger on which device, so can you please gimme a screenshot from your iPhone&Mac for the following:

// update if necessary
var button = document.getElementsByTagName('BUTTON')[0];

function testHandler(e){
  if (e.target === button){
    button.innerHTML = e.type;
    console.log(e.type)
  }
}
// iphone
document.addEventListener('touchstart',testHandler,false)

// iphone/mac
button.addEventListener('mouseout',testHandler,false)
button.addEventListener('mouseleave',testHandler,false)
button.addEventListener('focusout',testHandler,false)
button.addEventListener('blur',testHandler,false)

@cvaize
Copy link
Contributor Author

cvaize commented Jan 10, 2020

Okay, here's a video of the test: https://drive.google.com/file/d/1f1n-UEIDJbKN88rz9lllq7n9xw1-if8U/view?usp=sharing

@thednp
Copy link
Owner

thednp commented Jan 10, 2020

So it looks like the events barely fire, like I imagined they would.

thednp added a commit that referenced this issue Jan 18, 2020
* addressing #347, @iangregory please check master
* addressing #345, added `touch` handlers to Tooltip and Popover, @cvaize this is on you
* addressing #346 & #335, I've added a "hard kill" `dispose()` method to Carousel, but `slid` event will still trigger, so keep that in mind, @cvaize
@thednp thednp added enhancement potential improvement and removed bug critical issue labels Jan 18, 2020
@cvaize
Copy link
Contributor Author

cvaize commented Jan 18, 2020

@thednp
Copy link
Owner

thednp commented Jan 18, 2020

Did you test the latest master? There have been some changes since that initial commit. On the way I got those errors too.

@cvaize
Copy link
Contributor Author

cvaize commented Jan 18, 2020

In the video, I demonstrated the commits.
49f14d6

@thednp
Copy link
Owner

thednp commented Jan 18, 2020

Alright man, will check those asap. Thanks for the feedback.

thednp added a commit that referenced this issue Feb 2, 2020
* moved all components into a new folder `/src/components`
* some optimizations as well as code improvements with `.map()` and such, will look into more soon
* fixing #347 for ScrollSpy please test @iangregory
* fixing #345 for Tooltip and Popover @cvaize please test 
* fixing #346 and #335 @cvaize special, this time I think it's pretty much as far as we can go
@thednp
Copy link
Owner

thednp commented Feb 9, 2020

@cvaize how we doin?

@thednp
Copy link
Owner

thednp commented Feb 15, 2020

@cvaize I will start documenting V4 next week and I really need your feedback.

@cvaize
Copy link
Contributor Author

cvaize commented Feb 17, 2020

Sorry for the delay. I'll run tests now

@cvaize
Copy link
Contributor Author

cvaize commented Feb 17, 2020

I ran tests. Bug with Carousel fixed. The current bug in iphone 5 related to Popover and Tooltip remained, I think it is not critical and you can design documentation

@thednp
Copy link
Owner

thednp commented Feb 17, 2020

@cvaize I put some time into the tooltip/popover issue and I still need help in finding a solution.

@cvaize
Copy link
Contributor Author

cvaize commented Feb 18, 2020

@thednp Since I have an iphone 5 I will look for a solution, leave this issue to me. This problem is important,but it is not urgent. I will look for a solution to this problem on the weekend

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

Here's what I found, an error in registering a touch listener
image
image
image

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

image

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

After correctly binding the listener, the tooltip started closing correctly.
image

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

But, re-opening the tooltip on the same button does not happen, I am investigating

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

https://drive.google.com/file/d/1gE2wciRp-oRm-C-n-sGYjM0rUfNhXMRC/view?usp=sharing
@thednp Be sure to watch this video, this is very important when working with all Iphone versions

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

I will prepare a pull request for correction

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

Working on Popover, I noticed a difference in the work of Popover from the original library, Popover trigger=click https://drive.google.com/open?id=1FLgiZiXb1_USsaluczdzjteulRhAlZrg

@cvaize
Copy link
Contributor Author

cvaize commented Feb 22, 2020

On iphone, the focus event doesn't work as it should. I want to use the voiced solution jquery-archive/jquery-mobile#3016 (comment)
I checked it works, but there is an important question.
I need detect mobile, but only 1 time. At bootstrap.native there are global variables, if not then how should they be implemented? Or should I make a pull request for existing edits with Tooltip and Popover for Hover and Click events, and you will fix the Focus event yourself?

cvaize added a commit to cvaize/bootstrap.native that referenced this issue Feb 22, 2020
… events on mobile devices, in particular Iphone 5

thednp#345
@thednp
Copy link
Owner

thednp commented Feb 23, 2020

That's an interesting find because on my Android, Chrome works as expected as is, without your modifications. Please send me your mobile navigator string, I will give you a solution to your issue.

@cvaize
Copy link
Contributor Author

cvaize commented Feb 23, 2020

User Agent: Mozilla/5.0 (iPhone; CPU iPhone OS 10_3_4 like Mac OS X) AppleWebKit/603.3.8 (KHTML, like Gecko) Version/10.0 Mobile/14G61 Safari/602.1

@cvaize
Copy link
Contributor Author

cvaize commented Feb 23, 2020

Of course, it can be determined by user agent, but there is no guarantee that this will not become a problem in the future. I have a solution for user agent
/(Android|BlackBerry|iPhone|iPod|Palm|Symbian)/.test(navigator.userAgent)

@cvaize
Copy link
Contributor Author

cvaize commented Feb 23, 2020

Should I use it?

@thednp
Copy link
Owner

thednp commented Feb 23, 2020

Since our problem is strictly on Apple Safari mobile, this is the line

/(iPhone|iPod|iPad)/.test(navigator.userAgent)

@thednp
Copy link
Owner

thednp commented Feb 23, 2020

Wow Safari 17% market share.

@cvaize
Copy link
Contributor Author

cvaize commented Feb 23, 2020

Do you have an Iphone 10 or Iphone 11 or Ipad of the latest version, please check if Focus Event doesn't work on them either? This is necessary to check whether all mobile safaris have this problem or not.

@thednp
Copy link
Owner

thednp commented Feb 23, 2020

I wouldn't have asked for your help if I had, probably never will. I wish I had more community input.

@thednp
Copy link
Owner

thednp commented Feb 23, 2020

Listen, have it work fine on your device, I believe the fix will work with other Apple Safari mobiles as well. If somebody has a problem he can simply come and raise an issue and contribute with a fix.

@cvaize
Copy link
Contributor Author

cvaize commented Feb 23, 2020

Okay, I'll do a pull request

cvaize added a commit to cvaize/bootstrap.native that referenced this issue Feb 23, 2020
cvaize added a commit to cvaize/bootstrap.native that referenced this issue Feb 23, 2020
cvaize added a commit to cvaize/bootstrap.native that referenced this issue Feb 23, 2020
thednp added a commit that referenced this issue Feb 23, 2020
(Fix) Incorrect operation of Tooltip and Popover with focus events on mobile devices, in particular Iphone 5 #345
@thednp
Copy link
Owner

thednp commented Feb 23, 2020

So we good to finally close this at long last?

@cvaize
Copy link
Contributor Author

cvaize commented Feb 23, 2020

Yes

@cvaize cvaize closed this as completed Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement potential improvement
Projects
None yet
Development

No branches or pull requests

2 participants