Skip to content

Conversation

@iShibi
Copy link
Contributor

@iShibi iShibi commented Sep 13, 2021

Please describe the changes this PR makes and why it should be merged:

Starts work on role icons:

Status and versioning classification:

  • I know how to update typings and have done so
  • This PR changes the library's interface (methods or parameters added)

Copy link
Contributor

@ImRodry ImRodry left a comment

Choose a reason for hiding this comment

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

Small nitpicks as I know this is still a WIP

Copy link
Contributor

@NotSugden NotSugden left a comment

Choose a reason for hiding this comment

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

can't you also pass null for the role icon? if so docs should be updated

@Lulalaby
Copy link

can't you also pass null for the role icon? if so docs should be updated

Yes u can

@ImRodry
Copy link
Contributor

ImRodry commented Sep 21, 2021

New upstream PR: discord/discord-api-docs#3847

@iShibi iShibi marked this pull request as ready for review September 22, 2021 04:55
@iShibi
Copy link
Contributor Author

iShibi commented Sep 22, 2021

This is ready for review ✨

@iCrawl iCrawl added this to the Version 13.2 milestone Sep 23, 2021
@Lulalaby
Copy link

hi so i guess it's worth mentioning that, so i'm using this and if I .setIcon() to an animated emoji, absolutely nothing happens, not even an error. But in discord ui you can pick an animated emoji and it'll use the first frame.

yeah, that's a client thing.
But yeah, discord seems like not to throw errors

@ImRodry
Copy link
Contributor

ImRodry commented Sep 29, 2021

hi so i guess it's worth mentioning that, so i'm using this and if I .setIcon() to an animated emoji, absolutely nothing happens, not even an error. But in discord ui you can pick an animated emoji and it'll use the first frame.

Well the fact that the url property on an emoji is a getter and not a function like it is everywhere else makes it hard to add a fix for this. We could just get the png image by calling the CDN endpoint directly because that way we’d always make sure to get the first frame of the emoji

@ghost
Copy link

ghost commented Sep 29, 2021

pardon my stupidity (i was actually setting it to undefined); it works fine. well transparency is lost but yeah

Copy link
Contributor

@advaith1 advaith1 left a comment

Choose a reason for hiding this comment

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

unicode_emoji is now the actual emoji, not a text name

@iCrawl
Copy link
Member

iCrawl commented Oct 2, 2021

This needs a rebase.

@iCrawl
Copy link
Member

iCrawl commented Oct 3, 2021

This needs a rebase.

@iShibi
Copy link
Contributor Author

iShibi commented Oct 3, 2021

Rebased 😓

@iCrawl iCrawl merged commit 7129965 into discordjs:main Oct 3, 2021
@iShibi iShibi deleted the feat-role-icon branch January 12, 2022 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.