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

feat(modal): modal service wip #2047

Merged
merged 26 commits into from Jul 18, 2017
Merged

feat(modal): modal service wip #2047

merged 26 commits into from Jul 18, 2017

Conversation

IlyaSurmay
Copy link
Contributor

@IlyaSurmay IlyaSurmay commented Jun 8, 2017

WIP
fixes #1998
fixes #1995
fixes #1830
fixes #1181
fixes #579

todo:

  • backdrop click to the left\right from dialog, doesn't close modal
  • nested modals
  • isAnimated (on and off) should work

@codecov
Copy link

codecov bot commented Jun 14, 2017

Codecov Report

Merging #2047 into development will decrease coverage by 0.46%.
The diff coverage is 65.38%.

Impacted file tree graph

@@              Coverage Diff               @@
##           development   #2047      +/-   ##
==============================================
- Coverage        87.16%   86.7%   -0.47%     
==============================================
  Files               85      85              
  Lines             2267    2459     +192     
  Branches           293     334      +41     
==============================================
+ Hits              1976    2132     +156     
- Misses             188     213      +25     
- Partials           103     114      +11
Impacted Files Coverage Δ
src/component-loader/component-loader.factory.ts 100% <100%> (ø) ⬆️
src/component-loader/component-loader.class.ts 78.33% <60.86%> (-12.3%) ⬇️
src/positioning/ng-positioning.ts 62.92% <0%> (-4.5%) ⬇️
src/popover/popover.directive.ts 95.58% <0%> (-2.04%) ⬇️
src/progressbar/progressbar.component.ts 100% <0%> (ø) ⬆️
src/popover/popover-container.component.ts 100% <0%> (ø) ⬆️
src/progressbar/bar.component.ts 93.18% <0%> (+1.18%) ⬆️
src/typeahead/typeahead.directive.ts 79.83% <0%> (+3.56%) ⬆️
src/tooltip/tooltip.directive.ts 74.62% <0%> (+5.8%) ⬆️

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 41f11e6...16760cb. Read the comment docs.

@YevheniiaMazur
Copy link

Tested. Found next issues:

  1. When the modal is open, the scroll continues to work
  2. Modal from service described as static in its body, but it's not static.

@YevheniiaMazur
Copy link

Tested. Looks good now

Conflicts:
	demo/src/app/components/+modal/demos/index.ts
	demo/src/app/components/+modal/modal-section.component.ts
@SergeyKuryatnick
Copy link
Contributor

Tested, looks good.

Notice: two unit test are failed

@valorkin valorkin merged commit 2d02faa into development Jul 18, 2017
@valorkin valorkin deleted the feat-modal-service branch July 18, 2017 14:18
@albohlabs
Copy link

Awesome work. Can't wait to refactor my code.
How long do we need to wait for the release on npm? ⏰

@valorkin
Copy link
Member

one more PR to merge
new version complete QA testing
and than release

@valorkin
Copy link
Member

later today or tomorrow

@volkanokcu
Copy link

Hi! How can i encapsule my modal component. I dont want to write moal header body.. other element in my component templete. I am using components normal pages. But sometimes i need to open them in modal other pages. I did its working well. But look like ugly. I want to encapsule my component templete another templete. Open component inside another templete. How can i do that?

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