Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Added handling for disabling iframes when a flyout is being dragged #1952

Merged
merged 4 commits into from
Sep 5, 2018

Conversation

Blackbaud-TrevorBurch
Copy link
Member

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch commented Aug 28, 2018

Original code basis from @Blackbaud-StewartStephens

Resolves #1794

@Blackbaud-TrevorBurch
Copy link
Member Author

@codecov-io
Copy link

codecov-io commented Aug 28, 2018

Codecov Report

Merging #1952 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1952   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         424     424           
  Lines        8968    8974    +6     
  Branches     1326    1327    +1     
======================================
+ Hits         8968    8974    +6
Impacted Files Coverage Δ
src/modules/flyout/flyout.component.ts 100% <100%> (ø) ⬆️
src/modules/flyout/flyout-adapter.service.ts 100% <100%> (ø) ⬆️

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 de8ad15...8d30b29. Read the comment docs.

@@ -37,4 +37,15 @@ export class SkyFlyoutAdapterService {
this.renderer.addClass(header.nativeElement, 'sky-flyout-help-shim');
}
}

public toggleIframePointerEvents(enable: boolean): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call this enableIframePointerEvents to be more specific?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. As enable is a flag that enables or disables. I feel like toggle is more specific as it shows that you can toggle on or off. enable sounds to me like it is always turning it on

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... not sure I agree there

https://media.giphy.com/media/EVbEdEW3kuu0o/giphy.gif

But since you've been a good sport with all my other PR comments today, I'll let this one slide :) The code is sound 👍

Copy link
Contributor

@Blackbaud-AlexKingman Blackbaud-AlexKingman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 🇮🇹

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch merged commit 39cb87e into master Sep 5, 2018
@Blackbaud-TrevorBurch Blackbaud-TrevorBurch deleted the 1794-flyout-iframe-dragging branch September 5, 2018 19:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants