sergiitk / pagerbeauty

:pager::sparkles: PagerDuty on-call widget for monitoring dashboard. Datadog and Grafana compatible

https://demo.pagerbeauty.sergii.org/v1/schedules/PDUQGYJ.html

MIT License

338 stars 27 forks source link

Open Links To PagerDuty In New Tabs #161

Closed JamesFrost closed 3 years ago

JamesFrost commented 3 years ago

What's this PR do?

When embedding a schedule in an iframe, the PagerDuty links are not able to be opened. This is because PagerDuty sets the 'X-Frame-Options' header to 'sameorigin', which effectively stops the browser from rending the page in a iframe.

As such, it's more appropriate to open these links in a new tab.

Iframe before clicking PagerDuty link:

Iframe after clicking PagerDuty link:

Console error:

Refused to display 'https://apidocs.pagerduty.com/' in a frame because it set 'X-Frame-Options' to 'sameorigin'.

Where should the reviewer start?

N/A

How should this be manually tested?

Do the PagerDuty links open in a new tab?

Any background context you want to provide?

N/A

What are the relevant tickets?

N/A

codecov[bot] commented 3 years ago

Codecov Report

Merging #161 (a7c3d00) into master (a9f49da) will not change coverage. The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #161   +/-   ##
======================================
  Coverage    7.62%   7.62%           
======================================
  Files          25      25           
  Lines         538     538           
======================================
  Hits           41      41           
  Misses        497     497

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 a9f49da...a7c3d00. Read the comment docs.

avanier commented 3 years ago

Yes, this, please. Please merge this. :scream:

Thanks for the PR! :two_hearts:

JamesFrost commented 3 years ago

@avanier You're welcome 😎

@sergiitk Anything we can do to get this merged?

sergiitk commented 3 years ago

Hey folks,

Was moving to another state, didn't even have a chair until yesterday :) Sorry for the delay. Will get it merged today. And publish Docker dev image. I'll try to make a stable release next week, but can't promise it.

With best regards, Sergii

sergiitk commented 3 years ago

This is in now the latest sergiitk/pagerbeauty:dev Docker image. Deployed to Live Demo as well: https://demo.pagerbeauty.sergii.org/v1/schedules.html