added props and events aligned with latest electron version 1.4.8 #10
Conversation
|
Hi @keokilee can you look at my PR and possible merge it ? |
| @@ -28,21 +32,32 @@ export interface WebViewProps extends React.HTMLAttributes<Electron.WebViewEleme | |||
| onDidGetResponseDetails?: EventListener | |||
| onDidGetRedirectRequest?: EventListener | |||
| onDomReady?: EventListener | |||
| onPageTitleSet?: EventListener | |||
| onPageTitleUpdated?: EventListener | |||
There was a problem hiding this comment.
Do you know when this was changed? I'm thinking we leave them both in for compatibility, but with a comment to remove it later.
There was a problem hiding this comment.
They changed this in latest electron api.
There was a problem hiding this comment.
Okay, then we should leave them both in for compatibility. I don't think it's worth bumping up our Electron peer dependency just for this.
|
|
||
| autosize?: boolean | ||
|
|
||
| autosize?: string; |
There was a problem hiding this comment.
Does this need to be a string? Seems like it is in the Electron docs but I'm pretty sure just the presence of this attribute will trigger it.
There was a problem hiding this comment.
On official docs, it says: setting 'on' to enable.
There was a problem hiding this comment.
Yeah, but it says that for all of the boolean attributes like nodeintegration and plugins, so I'm fairly certain it behaves like a boolean where the actual value doesn't matter. It just matters if the attribute is there or not.
| @@ -12,18 +12,29 @@ const EVENTS = [ | |||
| 'did-get-response-details', | |||
| 'did-get-redirect-request', | |||
| 'dom-ready', | |||
| 'page-title-set', | |||
| 'page-title-updated', | |||
There was a problem hiding this comment.
Same as above. I'm leaning towards keeping both.
| @@ -76,7 +87,7 @@ export default class WebView extends React.Component { | |||
| } | |||
|
|
|||
| const tagPropTypes = { | |||
| autosize: React.PropTypes.bool, | |||
| autosize: React.PropTypes.string, | |||
There was a problem hiding this comment.
Same as above. Should probably test that this can be a boolean and not be a string. Although if it's too confusing, I'm okay with it being a union type.
|
@keokilee: I found now that reactjs rejecting those props as not supported :-( Dont know how to tell reactjs to accept those props. |
to align against latest typescript definitions of electron version to v1.4.8 (from @types/electron)