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.

Sign up for GitHub

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

Jump to bottom

feat(Message|TextChannel): Inline replies #4874

Merged
merged 25 commits into from
Dec 8, 2020
Merged

feat(Message|TextChannel): Inline replies #4874

merged 25 commits into from
Dec 8, 2020
+80 −81

Conversation

monbrey
Copy link
Member

@monbrey monbrey commented Oct 2, 2020 •
edited
Loading

Ref: discord/discord-api-docs#2118

This implements the inline reply functionality, both for receiving and sending messages.
It now also steals merges the changes that Souji had made in #4870 to remove Message#reply and instead completely replaces it with inline reply functionality under the same name. It is therefore semver major

Changes

  • MessageOptions#replyTo: MessageResolvable added
  • MessageOptions#reply removed
  • Message#reply modified to use inline-reply functionality
  • APIMessage#resolveData updated to resolve replyTo parameters
  • Typings updated accordingly
  • Boilerplate, examples and tests updated thanks to @almostSouji

Status

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

NotSugden
NotSugden suggested changes Oct 2, 2020
View reviewed changes
src/structures/Message.js Outdated Show resolved Hide resolved
src/structures/Message.js Outdated Show resolved Hide resolved
@almostSouji almostSouji mentioned this pull request Oct 7, 2020
Closed
5 tasks
@monbrey monbrey changed the title feat: inline replies [WIP] feat(Message|TextChannel}: Inline replies Oct 7, 2020
@almostSouji almostSouji mentioned this pull request Oct 7, 2020
Closed
5 tasks
tguichaoua
tguichaoua suggested changes Oct 7, 2020
View reviewed changes
src/structures/Message.js Outdated Show resolved Hide resolved
@monbrey @tguichaoua
fix: jsdocs for Message#referencedMessage
0828645 
Co-authored-by: Tristan Guichaoua <33934311+tguichaoua@users.noreply.github.com>
@monbrey monbrey changed the title feat(Message|TextChannel}: Inline replies feat(Message|TextChannel): Inline replies Oct 7, 2020
@advaith1
Copy link
Contributor

advaith1 commented Oct 28, 2020 •
edited
Loading

we should add a mention/ping option to mention the author of the original message (replied_user boolean in allowed_mentions, true by default)

the mention is no longer present at the start of the message content when replying with pinging

image

gif

vladfrangu
vladfrangu requested changes Nov 3, 2020
View reviewed changes
README.md Show resolved Hide resolved
@oddyamill

This comment has been minimized.

Sign in to view
@hackermondev

This comment has been minimized.

Sign in to view
@kylerchin
Copy link
Contributor

Is this ready to be merged into master?

@monbrey
Copy link
Member Author

monbrey commented Nov 27, 2020

Is this ready to be merged into master?

No

@vico93 vico93 mentioned this pull request Nov 27, 2020
Closed
This was referenced Nov 27, 2020
Closed
Closed
@vaporoxx vaporoxx mentioned this pull request Dec 2, 2020
Closed
5 tasks
@kyranet kyranet linked an issue Dec 6, 2020 that may be closed by this pull request
Implementation of new Replies functionality #5071
Closed
vladfrangu
vladfrangu approved these changes Dec 7, 2020
View reviewed changes
Copy link
Member

@vladfrangu vladfrangu left a comment

Choose a reason for hiding this comment

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

Looks fine to me! LGTM 👍

@NotSugden
Copy link
Contributor

the only use of the APIMessage#isUser getter is removed in this PR, it would make sense if this PR also removed that getter

@iCrawl iCrawl merged commit 60e5a0e into discordjs:master Dec 8, 2020
@hackermondev

This comment has been minimized.

Sign in to view
@StepS- StepS- mentioned this pull request Dec 12, 2020
Open
@almostSouji almostSouji mentioned this pull request Jan 9, 2021
Closed
@xiBread xiBread mentioned this pull request Jan 20, 2021
Closed
@xiBread xiBread mentioned this pull request Mar 6, 2021
Closed
@monbrey monbrey deleted the inline-replies branch May 2, 2021 21:38
@iCrawl iCrawl added this to the Version 13 milestone Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mome0320 mome0320 mome0320 left review comments

@almostSouji almostSouji almostSouji left review comments

@NotSugden NotSugden NotSugden requested changes

@tguichaoua tguichaoua tguichaoua requested changes

@vladfrangu vladfrangu vladfrangu approved these changes

@iCrawl iCrawl iCrawl approved these changes

@kyranet kyranet kyranet approved these changes

@SpaceEEC SpaceEEC SpaceEEC approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Version 13
Development

Successfully merging this pull request may close these issues.

Implementation of new Replies functionality