Skip to content

Conversation

@PatrickJS
Copy link
Contributor

  • Please check if the PR fulfills these requirements
  • The commit message follows our guidelines
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    allows the user to bind to global events using the "global" name as an alias to "window"

@jeffbcross
Copy link
Contributor

I don't understand why the browser platform should/would ever have a global object called global? And in this case, the reference isn't to some injected Global.

The tokens being added with #7438 are only for use with DI.

@jeffbcross jeffbcross added comp: core action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 8, 2016
@jeffbcross jeffbcross added action: discuss and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 8, 2016
@PatrickJS
Copy link
Contributor Author

The idea is to have a common language for window as global or at least support both names. For example we can bind to a global click event via <div (window:click)="onEvent($event)"> when something like <div (global:click)="onEvent($event)"> should also work. This PR allows for both names being a reference to window/global.

But only if we consider that the name global, for an application, should be window (or at least a fake window context) otherwise it does make sense to keep the name "window" (assuming window represents the global application context).

 ___________________________
|global/node/server         |
|    ______________________ |   
|   |window/index.html     ||
|   |    ________________  ||
|   |   | universal/app  | ||
|   |   |________________| ||
|   |    ________________  ||
|   |   | universal/app  | ||
|   |   |________________| ||
|   |    ________________  ||
|   |   | universal/app  | ||
|   |   |________________| ||
|   |______________________||
|___________________________|

If we create a Node.js context for each Angular application in then each context will be considered as window in that context. Since we're adding an eventListener to window (or the node.js context) rather than node's global.

@PatrickJS PatrickJS mentioned this pull request Apr 8, 2016
1 task
@mhevery mhevery assigned mhevery and unassigned PatrickJS Apr 17, 2016
@mhevery
Copy link
Contributor

mhevery commented Apr 17, 2016

@gdi2290 I am a bit confused about this change as well. I don't see the need to have two different ways to refer to the same thing.

My default position would be no. I would need more context as to why this would be so.

Also we are planning to get rid of BrowserAdapter and instead have different Renderer implementations.

@mhevery
Copy link
Contributor

mhevery commented May 20, 2016

Closing due to inactivity.

@mhevery mhevery closed this May 20, 2016
@PatrickJS PatrickJS deleted the window-or-global branch May 20, 2016 03:56
@PatrickJS
Copy link
Contributor Author

I forgot to close this. window is better for what it's doing

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
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.

4 participants