Skip to content

Conversation

@parro-it
Copy link
Owner

@parro-it parro-it commented Dec 9, 2018

  • I add tslint package to lint typescript defs file.
  • Changed syntax for nested enums from static property to namespace to fix TS error.
  • Manually add getter setters (as @mischnic said, mapped properties cannot be used for this)

While doing these changes, I also find some error in type of properties that I'm going to fix with further commits here.

@mischnic
Copy link
Contributor

mischnic commented Dec 9, 2018

Unfortunately, clang-format doesn't really work well with typescript

/**
* Create a new UiControl object.
*/
constructor(handle: any);
Copy link
Contributor

@mischnic mischnic Dec 10, 2018

Choose a reason for hiding this comment

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

This is the last remaining "private/internal" constructor that should be removed from the public API definition

Copy link
Owner Author

Choose a reason for hiding this comment

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

Why you said constructor? Anyway, I never exactly understand how that could be used, apart for read. Do you never try to set toplevel=false on a toplevel control?

Copy link
Contributor

Choose a reason for hiding this comment

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

No 😄 , I meant this part:

libui-napi/libui-napi.d.ts

Lines 354 to 357 in dade525

/**
* Create a new UiControl object.
*/
constructor(handle: any);

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah, sure! I guess it should be protected?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just remove it, because libui-napi (js/*.js) itself isn't typechecked. This is just the public API.

Copy link
Contributor

@mischnic mischnic Dec 10, 2018

Choose a reason for hiding this comment

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

And toplevel is just a getter, so it should be readonly: (the documentation is wrong as well)

/**
* Set or return whether the control is a top level one.
* @return {boolean}
*/
get toplevel() {
return ControlBase.toplevel(this.handle);
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

That explains my previous doubt on its being writable 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants