Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Typescript 2.6.1 #774

Merged
merged 6 commits into from Nov 30, 2017
Merged

Typescript 2.6.1 #774

merged 6 commits into from Nov 30, 2017

Conversation

bryanforbes
Copy link
Member

Type: enhancement

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

Relates to dojo/meta#210

export interface WidgetAndElementEvent extends EventTypedObject<'properties:changed'> {
key: string;
element: HTMLElement;
export interface WidgetAndElementEvent<T extends EventType> extends EventObject<T> {
Copy link
Member

Choose a reason for hiding this comment

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

Don't think this get used anymore?

@@ -78,7 +78,7 @@ export class WidgetBase<P = WidgetProperties, C extends DNode = DNode> implement
/**
* Array of property keys considered changed from the previous set properties
*/
private _changedPropertyKeys: string[] = [];
private _changedPropertyKeys: (string | number)[] = [];
Copy link
Member

Choose a reason for hiding this comment

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

This should just be string, it is the actual key of the property on the properties object

// Do nothing by default.
}

public get properties(): Readonly<P> & Readonly<WidgetProperties> {
return this._properties;
}

public get changedPropertyKeys(): string[] {
public get changedPropertyKeys(): (string | number)[] {
Copy link
Member

Choose a reason for hiding this comment

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

As above, this should just be string[]

Copy link
Member

Choose a reason for hiding this comment

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

And all the other references to changedPropertyKeys

@agubler
Copy link
Member

agubler commented Nov 27, 2017

Looks reasonable, just a conflict on vdom.ts now, you're welcome :)

@bryanforbes bryanforbes merged commit c6af722 into dojo:master Nov 30, 2017
@bryanforbes bryanforbes deleted the typescript-2.6.1 branch November 30, 2017 17:11
@kitsonk kitsonk added beta5 and removed beta4 labels Dec 1, 2017
@dylans dylans added this to the beta.5 milestone Dec 22, 2017
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.

None yet

4 participants