Skip to content
This repository was archived by the owner on May 22, 2020. It is now read-only.

Conversation

@MichaelMCoates
Copy link
Contributor

@MichaelMCoates MichaelMCoates commented Oct 2, 2019

Description of Change

This PR adds propagated events up to Application and System for BrowserView.

Tests:
https://testing-dashboard.openfin.co/#/app/sessions/api/completed/5da8b1ceee166d7a14d451bc

Corresponding core PR:
HadoukenIO/core#976

@aziz512 trying to add you as reviewer but it won't let me for some reason.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • automated tests are changed or added
  • relevant documentation is changed or added
  • Link to new tests added
  • PR has assigned reviewers

Release Notes

Notes:

@MichaelMCoates MichaelMCoates changed the title [N/A] Hook up event propagation for all browserview-related events DO NOT REVIEW YET [N/A] Hook up event propagation for all browserview-related events Oct 2, 2019
@openfin-github-bot openfin-github-bot bot added the auto testing started Bot started automated testing label Oct 2, 2019
@openfin-github-bot
Copy link

eae221f

Git

  • core: develop <= dev/coates/bv-event-propagation (c7afb49)
  • js-adapter: develop <= dev/coates/bv-event-propagation (eae221f)
  • javascript-adapter: develop

Asars used for testing

Test results

@openfin-github-bot openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 2, 2019
@aziz512 aziz512 self-requested a review October 9, 2019 23:22
@openfin-github-bot
Copy link

⚠️ Failed to build 87dedcf

@openfin-github-bot openfin-github-bot bot added auto testing failed Bot failed to build and removed auto testing done Bot completed automated testing labels Oct 10, 2019
@openfin-github-bot openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing failed Bot failed to build labels Oct 10, 2019
@openfin-github-bot
Copy link

f231be5

Git

  • core: develop <= dev/coates/bv-event-propagation (99ab0e7)
  • js-adapter: develop <= dev/coates/bv-event-propagation (f231be5)
  • javascript-adapter: develop

Asars used for testing

Test results

@openfin-github-bot openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 10, 2019
@MichaelMCoates MichaelMCoates changed the title DO NOT REVIEW YET [N/A] Hook up event propagation for all browserview-related events [N/A] Hook up event propagation for all browserview-related events Oct 11, 2019
dhamberlin
dhamberlin previously approved these changes Oct 11, 2019
'window-view-hidden': WindowEvent<Topic, Type>;
'window-view-page-favicon-updated': WindowEvent<Topic, Type>;
'window-view-page-title-updated': WindowEvent<Topic, Type>;
'view-resource-load-failed': WindowResourceLoadFailedEvent<Topic, Type>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

'window' is missing in the event name?

'window-view-page-favicon-updated': WindowEvent<Topic, Type>;
'window-view-page-title-updated': WindowEvent<Topic, Type>;
'view-resource-load-failed': WindowResourceLoadFailedEvent<Topic, Type>;
'view-resource-response-received': WindowResourceResponseReceivedEvent<Topic, Type>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

* window-start-load
* window-user-movement-disabled (see {@tutorial Window.EventEmitter})
* window-user-movement-enabled (see {@tutorial Window.EventEmitter})
* window-view-attached (see {@tutorial Window.EventEmitter})
Copy link
Collaborator

Choose a reason for hiding this comment

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

@tutorial Window.EventEmitter => @tutorial BrowserView.EventEmitter

* window-view-crashed (see {@tutorial BrowserView.EventEmitter})
* window-view-created (see {@tutorial BrowserView.EventEmitter})
* window-view-destroyed (see {@tutorial BrowserView.EventEmitter})
* window-view-detached (see {@tutorial Window.EventEmitter})
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All fixed. Thank you!

@openfin-github-bot openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing done Bot completed automated testing labels Oct 17, 2019
@openfin-github-bot
Copy link

ad6048c

Git

  • core: develop <= dev/coates/bv-event-propagation (bad0f85)
  • js-adapter: develop <= dev/coates/bv-event-propagation (ad6048c)
  • javascript-adapter: develop

Asars used for testing

Test results

@openfin-github-bot openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 17, 2019
@MichaelMCoates
Copy link
Contributor Author

MichaelMCoates commented Oct 17, 2019

After talking with Pierre and Tommy, changed the eventing scheme to the following:

  • Windows can listen to view-attached and view-detached.
  • Views can listen to target-changed.
  • Window view events don't propagate up to Application and System.
  • All view events propagate up to Window, Application, and System.

The tests have been updated to reflect these changes.

@openfin-github-bot openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing done Bot completed automated testing labels Oct 17, 2019
@openfin-github-bot
Copy link

2289f59

Git

  • core: develop <= dev/coates/bv-event-propagation (f877a7f)
  • js-adapter: develop <= dev/coates/bv-event-propagation (2289f59)
  • javascript-adapter: develop

Asars used for testing

Test results

@openfin-github-bot openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

auto testing done Bot completed automated testing cla-present

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants