-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Ponyfilling window.open #1704
Ponyfilling window.open #1704
Conversation
+ } | ||
+ }; | ||
+ } | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't really feel like a ponyfill, its more of a custom url click handler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced this should be about "ponyfilling" window.open. It looks to be more of allowing users to customize how to handle card clicks. For example, what if don't even want to open the link but instead do something else?
Given that, I think we should re-think the name.
If we name it "urlHandler" or similar, the user will have an impression that it could handle all But if we name it as Web Chat only use the first argument of |
We can name it "cardActionOpenUrlHandler". Or we make the whole
If we are working on |
I agree that the way this is written, full control is yielded to the caller and we would be presumptive that their implementation would always fall back to a closure that invokes |
@cwhitten If this is named |
Changed from window.open ponyfill into a full-fledged onCardAction middleware
@compulim nice work! let's rebase this and get it merged. |
e575493
to
35ec7ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Co-Authored-By: compulim <[email protected]>
Co-Authored-By: compulim <[email protected]>
204c623
to
632a59a
Compare
Background
To support Bot Framework Emulator and other hosting scenarios, all open URL actions should be customizable in JavaScript to allow different handling options.
Design decisions
Some design decisions have been made while preparing this work.
Function signature
window.open
has a signature of(url: string, windowName: string, windowFeatures: string) => void
.Instead of adopting a similar signature of
(url: string) => void
, we prefer({ cardAction: any }) => (url: string) => void
.This allow us to send in
cardAction
and additional data, and not messing up with current signature ofwindow.open
.Intercepting
<a>
Currently, we do not intercept
<a>
because it should be up to the hosting app on how to intercept them. And we believe the host should have ability to intercept them.Intercepting
<a>
is a challenge because interception might break middle-click on them (normally it should open the link in a new window.)OAuth support
Today, OAuth flow do the following:
window.open()
with nothing, remember the result aspopup
variablegetSessionId()
forcode_challenge
popup.location.href
with a URL withcode_challenge
embedded inThe 1st and 3rd point is required to be separated to overcome popup blocker restrictions. We are advised not to pre-fetch
code_challenge
until the user click on it.The function signature design allows us to do the job as of today, without intervention from popup blocker.
Changelog
Added
component
: Allow ponyfillingwindow.open
, by @compulim, in PR #1704Samples
component
: Customizing open URL behavior, in PR #1704