Skip to content
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

Xd 29 updates #77

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Xd 29 updates #77

wants to merge 15 commits into from

Conversation

pklaschka
Copy link
Contributor

Contents of this Pull Request

This PR makes changes to

  • The typings in the types folder
  • Configuration files (tsconfig.json, jsconfig.json, .gitignore etc.)
  • The sample files (e.g., sample.js)
  • The README.md or other files containing documentation
  • Other

Purpose of this Pull Request

To update the types to the current API status

Issues resolved by this Pull Request

@pklaschka pklaschka added the enhancement New feature or request label May 20, 2020
@pklaschka pklaschka self-assigned this May 20, 2020
pklaschka added 2 commits May 20, 2020 17:50
# Conflicts:
#	sample.js
#	types/application.d.ts
#	types/index.d.ts
#	types/interactions.d.ts
#	types/scenegraph.d.ts
@pklaschka
Copy link
Contributor Author

@ericdrobinson Would you mind taking a look at this? Thank you very much in advance 🙂

Copy link

@ericdrobinson ericdrobinson left a comment

Choose a reason for hiding this comment

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

The only major issue with this section is the broken @examples (at least for VSCode's IntelliSense service). The *s before each line within the example block are important, it turns out.

It also appears that the tripple-tick (```) syntax is optional. I've left it in with my suggestions to help clear things up.

I will also note that VSCode highlighted two unnecessary declare keywords in this file. They do not need to exist in front of individual classes as the module itself is already declared.

types/scenegraph.d.ts Outdated Show resolved Hide resolved
types/scenegraph.d.ts Outdated Show resolved Hide resolved
types/scenegraph.d.ts Outdated Show resolved Hide resolved
types/scenegraph.d.ts Outdated Show resolved Hide resolved
types/scenegraph.d.ts Outdated Show resolved Hide resolved
types/scenegraph.d.ts Outdated Show resolved Hide resolved
types/scenegraph.d.ts Outdated Show resolved Hide resolved
types/scenegraph.d.ts Outdated Show resolved Hide resolved
Copy link

@ericdrobinson ericdrobinson left a comment

Choose a reason for hiding this comment

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

Some formatting suggestions here.

types/application.d.ts Outdated Show resolved Hide resolved
types/application.d.ts Outdated Show resolved Hide resolved
types/application.d.ts Outdated Show resolved Hide resolved
types/index.d.ts Outdated Show resolved Hide resolved
types/index.d.ts Outdated Show resolved Hide resolved
types/index.d.ts Show resolved Hide resolved
pklaschka and others added 2 commits May 20, 2020 19:41
Co-authored-by: Eric Robinson <[email protected]>
Co-authored-by: Eric Robinson <[email protected]>
@ericdrobinson
Copy link

ericdrobinson commented May 20, 2020

@pklaschka I've done a first-pass review and hit the major points, I think. Basically:

  1. You might want to go through and do a pass on @example code to clean it up. I saw other locations throughout the code base with similar issues to those pointed out above.
  2. You might want to go through and remove the intra-documentation link formatting. It linkifies in the IntelliSense but shows an error if you click it.
  3. Adding BLEND_MODE_*s to the SceneNode base class definition causes it to appear in all subclasses. A different approach should be determined here. You might try resurrecting the SceneNode interface (and reverting all that SceneNodeClass stuff), add those properties to the SceneNode interface, and then declare a SceneNode name of a type that only contains the various "static properties" and export that.
    • Note that you do not want to export something like SceneNode: SceneNode because it would contain all the properties/methods of the SceneNode class which the actual SceneNode object you get in Xd from the scenegraph module has nothing public on it but standard Object stuff and the properties.

@ericdrobinson
Copy link

@pklaschka With respect to the third item above, something like this looks to work a treat:

// Define the shape of the SceneNode "restricted" static properties.
//  NOTE: These properties do not show up on sub-classes so they cannot
//  be a part of the class-inheritance tree.
interface SceneNodeStatic {
    readonly BLEND_MODE_PASSTHROUGH: string;
    readonly BLEND_MODE_NORMAL: string;
    readonly BLEND_MODE_MULTIPLY: string;
    readonly BLEND_MODE_DARKEN: string;
    readonly BLEND_MODE_COLOR_BURN: string;
    readonly BLEND_MODE_LIGHTEN: string;
    readonly BLEND_MODE_SCREEN: string;
    readonly BLEND_MODE_COLOR_DODGE: string;
    readonly BLEND_MODE_OVERLAY: string;
    readonly BLEND_MODE_SOFT_LIGHT: string;
    readonly BLEND_MODE_HARD_LIGHT: string;
    readonly BLEND_MODE_DIFFERENCE: string;
    readonly BLEND_MODE_EXCLUSION: string;
    readonly BLEND_MODE_HUE: string;
    readonly BLEND_MODE_SATURATION: string;
    readonly BLEND_MODE_COLOR: string;
    readonly BLEND_MODE_LUMINOSITY: string;
    readonly FIXED_LEFT: string;
    readonly FIXED_RIGHT: string;
    readonly FIXED_TOP: string;
    readonly FIXED_BOTTOM: string;
    readonly FIXED_BOTH: string;
    readonly POSITION_PROPORTIONAL: string;
    readonly SIZE_FIXED: string;
    readonly SIZE_RESIZES: string;
}

// Export the "name" of SceneNode. This provides access to the "restricted" static
//  properties.
// You could get the "type" of this by using `typeof SceneNode` (especially in type
//  declarations).
export const SceneNode: SceneNodeStatic;

// Export the "type" of SceneNode. This provides access to the "SceneNode" _type_.
export interface SceneNode extends SceneNodeBase {}

/**
 * Base class of all scenegraph nodes. Nodes will always be an instance of some subclass of SceneNode.
 */
declare abstract class SceneNodeBase {

NOTE: In the above, I renamed SceneNodeClass to SceneNodeBase as it better describes what its purpose is.

During my investigation I saw a few things that don't quite match up with this structure but it was nothing actually usable. Just differences in accessible prototype properties/functions. In terms of what is documented by Adobe and what you would want to use these APIs for, the structure I outline in this comment seems to work beautifully.

@pklaschka
Copy link
Contributor Author

Thank you so much for your input. I'm currently dealing with an ear infection (cf. my comment in the developer forum) and am unable to work on this, but I will look at it as soon as possible. Thank you again 👍.

@ericdrobinson
Copy link

Yikes! Take care and I wish you a swift recovery!

@pklaschka
Copy link
Contributor Author

@ericdrobinson:
You might want to go through and remove the intra-documentation link formatting. It linkifies in the IntelliSense but shows an error if you click it.

Truthfully, I wasn't able to replicate it. VSCode just doesn't link (at all) and WebStorm handled it fine (correctly linking). Do you have any examples where it wasn't working for you (I know it's been quite some time since your comment, sorry about that 😉 )?

@ericdrobinson
Copy link

Truthfully, I wasn't able to replicate it. VSCode just doesn't link (at all) and WebStorm handled it fine (correctly linking). Do you have any examples where it wasn't working for you (I know it's been quite some time since your comment, sorry about that 😉 )?

Yes. If you attempt to use scenegraph.getNodeByGUID() in VSCode, the IntelliSense will appear like so:

image

Note that "guid getter" is shown as a link in VSCode's IntelliSense view. If you click that link, you get the following error:

image

The links do not work.

It turns out that intra-API linking in IntelliSense is not supported by the TypeScript language server. This TypeScript issue is tracking interest/development.

While this does appear to work in WebStorm (and other JetBrains IDEs), any IDE that relies on the TypeScript Language Server (most non-JetBrains IDEs?) will report errors when you attempt to use this syntax.

That all said, the workaround would be to point to the Adobe XD documentation website. In the case outlined above, this would mean converting this:

[`guid` getter](#SceneNode-guid)

to this:

[`guid` getter](https://adobexdplatform.com/plugin-docs/reference/scenegraph.html#SceneNode-guid)

In my tests, this appears to work perfectly. Clicking that link opens the expected dialog in VSCode:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update typings to XD 29
2 participants