Skip to content

feat: match role embed color to role color for single role inputs#128

Open
SupaSeeka wants to merge 1 commit intoScottyLabs:mainfrom
SupaSeeka:feat/role-embed-colors
Open

feat: match role embed color to role color for single role inputs#128
SupaSeeka wants to merge 1 commit intoScottyLabs:mainfrom
SupaSeeka:feat/role-embed-colors

Conversation

@SupaSeeka
Copy link
Copy Markdown

Closes #127

The role embed colour now matches the role colour. I have tested this in my own test server with evidence here:
image

image image

/role members only takes the one string as an argument so no issue with multiple being added.

Copy link
Copy Markdown
Member

@TenType TenType left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! I've requested a few changes; please see below.

Comment on lines +78 to +82
const isSingleRole =
!roleString.match(/\b(AND|OR)\b/i) && !roleString.includes("(");
const singleRole = isSingleRole
? interaction.guild.roles.cache.find((r) => r.name === roleString)
: null;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const isSingleRole =
!roleString.match(/\b(AND|OR)\b/i) && !roleString.includes("(");
const singleRole = isSingleRole
? interaction.guild.roles.cache.find((r) => r.name === roleString)
: null;
const singleRole = interaction.guild.roles.cache.find(
(r) => r.name === roleString,
);

If more than one role is passed, then it won't be found in the cache anyways, so we can remove the first line and the ternary operator.

const singleRole = isSingleRole
? interaction.guild.roles.cache.find((r) => r.name === roleString)
: null;
const roleColor = singleRole?.color || null;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const roleColor = singleRole?.color || null;
const roleColor = singleRole?.colors.primaryColor;

color is deprecated, use colors.primaryColor instead.

Comment on lines 139 to 144
if (interaction.options.getSubcommand() === "count") {
const embed = new EmbedBuilder()
.setTitle(`"${roleString}"`)
.setDescription(`${members.length} members`);

return interaction.reply({ embeds: [embed] });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you also make this work for /role count?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Match role command embed colors to role colors

2 participants