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

feat(User): banners and accent colors #6117

Merged
merged 17 commits into from
Aug 24, 2021
Merged

feat(User): banners and accent colors #6117

merged 17 commits into from
Aug 24, 2021

Conversation

advaith1
Copy link
Contributor

@advaith1 advaith1 commented Jul 15, 2021

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

This PR is a continuation of #5930

Adds User#banner, User#bannerURL(), User#accentColor, and User#hexAccentColor (all require the user to be force fetched)

Since accent_color is currently only implemented in the canary API, you must add http: {headers: {'x-debug-options': 'canary'}} or http: {api: 'https://canary.discord.com/api'} to your ClientOptions to test accentColor/hexAccentColor

PR documenting this on Discord's end: discord/discord-api-docs#3448

Status and versioning classification:

  • 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
  • This PR changes the library's interface (methods or parameters added)

@advaith1 advaith1 changed the title Banners feat(User): banners and banner colors Jul 15, 2021
@iCrawl iCrawl added this to the Version 13 milestone Jul 15, 2021
@GoldenAngel2
Copy link
Contributor

banner and banner_color is now in the stable API

@advaith1
Copy link
Contributor Author

Discord will make banner_color an integer instead of a string to be consistent with other colors; this PR will be updated when that change is live

Copy link
Contributor

@xhyrom xhyrom left a comment

Choose a reason for hiding this comment

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

Wouldn't it be better to do it like this?

this.banner = {
      url: "url",
      color: "Hex"
}

@SuperchupuDev
Copy link
Contributor

Wouldn't it be better to do it like this?

this.banner = {
      url: "url",
      color: "Hex"
}

pretty sure it's not like that for consistency with other properties/methods

@xhyrom
Copy link
Contributor

xhyrom commented Jul 16, 2021

Wouldn't it be better to do it like this?

this.banner = {
      url: "url",
      color: "Hex"
}

pretty sure it's not like that for consistency with other properties/methods

So, you know, I say that because it would be good if it was the same everywhere. With embed footer you have footer.text, footer.iconURL and stuff. That's why I suggested this change.

@discordjs discordjs deleted a comment from Abdualml Jul 19, 2021
Copy link

@MoonLGH MoonLGH left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@iCrawl
Copy link
Member

iCrawl commented Aug 8, 2021

This needs a rebase.

@iCrawl iCrawl removed this from the Version 13.1 milestone Aug 13, 2021
@advaith1
Copy link
Contributor Author

upstream PR has been merged 🎉

@ImRodry
Copy link
Contributor

ImRodry commented Aug 16, 2021

Why doesn't this PR support banner_color?

@advaith1
Copy link
Contributor Author

because accent_color replaces banner_color, the old one will be removed

@ImRodry
Copy link
Contributor

ImRodry commented Aug 16, 2021

But it seems like one represents the HEX color and the other one is the base-10 color, are you sure it's gonna be removed?

src/structures/User.js Outdated Show resolved Hide resolved
@advaith1
Copy link
Contributor Author

But it seems like one represents the HEX color and the other one is the base-10 color, are you sure it's gonna be removed?

yes, banner_color was the original one but we got it changed to base 10 to be consistent with the other colors in the api

@iCrawl iCrawl added this to the Version 13.2 milestone Aug 17, 2021
@SpaceEEC SpaceEEC linked an issue Aug 23, 2021 that may be closed by this pull request
typings/index.d.ts Outdated Show resolved Hide resolved
iCrawl and others added 2 commits August 24, 2021 22:28
Co-authored-by: Rodry <38259440+ImRodry@users.noreply.github.com>
Co-authored-by: Rodry <38259440+ImRodry@users.noreply.github.com>
src/structures/User.js Outdated Show resolved Hide resolved
src/structures/User.js Outdated Show resolved Hide resolved
src/structures/User.js Outdated Show resolved Hide resolved
src/structures/User.js Outdated Show resolved Hide resolved
@iCrawl iCrawl merged commit 839c6da into discordjs:main Aug 24, 2021
@TechPro424
Copy link

Can't find banner in docs. Do docs need to be updated?

@ImRodry
Copy link
Contributor

ImRodry commented Aug 28, 2021

You probably just need to switch to the main branch

@casperiv0
Copy link
Contributor

Can't find banner in docs. Do docs need to be updated?

It's not in stable yet. You'll need to select the main branch in the docs settings. You can find it here

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.

User banner/accent color