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: Heatmap & domstream fixes #567

Merged
merged 11 commits into from Apr 29, 2020
Merged

BUG: Heatmap & domstream fixes #567

merged 11 commits into from Apr 29, 2020

Conversation

padams
Copy link
Collaborator

@padams padams commented Apr 28, 2020

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tested the changes
  • Build (/path/to/php cli.php cmd=build) was run locally and any changes were pushed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

  • Domstream module now uses the new REST API
  • Domstream playback window from iframe back to native window
  • Launching properly sized playback window directly from javascript
  • Fixing broken overlayLauncher with stopped launching heat maps due to Feature: Domstream screen resolution #534

Does this introduce a breaking change?

  • Yes
  • No

Docs need to be updated?

  • Yes
  • No

@padams padams added bug Bug in code confirmed and identified. REST API Domstreams labels Apr 28, 2020
@padams padams marked this pull request as ready for review April 28, 2020 21:38
@padams padams changed the title Heatmap & domstream fixes BUG: Heatmap & domstream fixes Apr 28, 2020
@padams padams requested a review from Maaiins April 28, 2020 21:56
@padams
Copy link
Collaborator Author

padams commented Apr 28, 2020

@Maaiins Your earlier commit in #534 broken heat maps as both heat maps and domstreams use the same overlayLauncher controller class. I refactored that by eliminating the need for overlayLauncher. Now we just spawn a new window (with the correct height/width) via javascript on the domstream report page.

.htaccess Show resolved Hide resolved
owa_auth.php Show resolved Hide resolved
owa_coreAPI.php Show resolved Hide resolved
@Maaiins
Copy link
Contributor

Maaiins commented Apr 29, 2020

@Maaiins Your earlier commit in #534 broken heat maps as both heat maps and domstreams use the same overlayLauncher controller class. I refactored that by eliminating the need for overlayLauncher. Now we just spawn a new window (with the correct height/width) via javascript on the domstream report page.

Shit didn´t recognized that. Was working fine when i tested it...

var width = jQuery(this).data('width');

var windowFeatures = "menubar=yes,location=yes,resizable=no,scrollbars=yes,status=yes,height=" + height + ",width=" + width;
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn´t came up with that idea, very nice

Copy link
Contributor

@Maaiins Maaiins left a comment

Choose a reason for hiding this comment

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

Approved

@padams padams merged commit 1b37826 into master Apr 29, 2020
@padams padams mentioned this pull request Apr 29, 2020
11 tasks
@padams padams deleted the heatmap_domstream_fixes branch April 29, 2020 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug in code confirmed and identified. Domstreams REST API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Heatmap feature is broken: No value passed. heatmap - not working in any mac browser
2 participants