-
Notifications
You must be signed in to change notification settings - Fork 662
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
types: Add "slack_file" object to "image" block/element types #1783
Conversation
This update adds the newly available slack_file object to the imageBlock and sets the image_url to be optional
…ImageBlock and ImageElement The new slack_file object must contain and id or a url so we use a union type to ensure either id or url is used. ImageBlock and ImageElement have also been updated. ImageBlock has been changed to a type so a union can be used
Co-authored-by: Fil Maj <[email protected]>
Co-authored-by: Fil Maj <[email protected]>
Co-authored-by: Fil Maj <[email protected]>
Co-authored-by: Fil Maj <[email protected]>
Co-authored-by: Fil Maj <[email protected]>
Thanks for the contribution! Before we can merge this, we need @hi1280 to sign the Salesforce Inc. Contributor License Agreement. |
The following pull request has not been updated for a while, so I have made an additional fix. |
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 great, thank you for the PR! I left two minor comments, just need to slightly move a couple of JSDocs but LGTM otherwise!
slack_file: SlackFile; | ||
} | ||
|
||
/** |
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.
Let's move this JSDoc comment right above the url
property, so that devs with Type/JavaScript language server support see the doc and link directly in their IDE!
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 have fixed this. Please confirm.
interface SlackFileViaUrl { | ||
url: string; | ||
} | ||
/** |
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.
Same here: let's move this JSDoc directly above the id
property.
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 have fixed this. Please confirm.
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.
LGTM! Thank you so much!
FYI the fix for this has been released to npm as |
Summary
Add "slack_file" object to the types package.
fix: #1734, #1738
Requirements (place an
x
in each[ ]
)