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

Popover focus on Mac OS fix #2031

Merged
merged 2 commits into from
Jul 14, 2017
Merged

Conversation

gilsdav
Copy link
Contributor

@gilsdav gilsdav commented Jun 3, 2017

@codecov
Copy link

codecov bot commented Jun 3, 2017

Codecov Report

Merging #2031 into development will decrease coverage by 0.1%.
The diff coverage is 25%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #2031      +/-   ##
===============================================
- Coverage        87.16%   87.05%   -0.11%     
===============================================
  Files               85       85              
  Lines             2259     2263       +4     
  Branches           292      292              
===============================================
+ Hits              1969     1970       +1     
- Misses             187      190       +3     
  Partials           103      103
Impacted Files Coverage Δ
src/popover/popover.directive.ts 91.3% <25%> (-6.32%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c172423...b06059e. Read the comment docs.

@gilsdav
Copy link
Contributor Author

gilsdav commented Jun 3, 2017

@valorkin What's wrong ?

@valorkin
Copy link
Member

why popover needs focus?

@gilsdav
Copy link
Contributor Author

gilsdav commented Jul 3, 2017

We need to hide it when clicking on other part of page. To be able to have blur event, we need focus before (like all Windows browser does but not some OSX browers). With this correction, it works like base bootstrap component (jquery).

@SergeyKuryatnick
Copy link
Contributor

Tested, looks good

@valorkin valorkin merged commit d039a8d into valor-software:development Jul 14, 2017
@valorkin
Copy link
Member

Thanks for PR!

GulajavaMinistudio added a commit to GulajavaMinistudio/ngx-bootstrap that referenced this pull request Jul 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants