Skip to content

Conversation

@dlchamp
Copy link
Contributor

@dlchamp dlchamp commented Jan 2, 2023

Description

Updates the page at /interactions/select-menu with relevant code examples and information about using the various select menus within disnake.

Relevant Issues

Closes #53

- Correct a couple spelling mistakes
- added a bit more detail to some of the comments and wording to better get the points across and provide more detail
- also removed an errant `command_sync_flags` I was using during testing the code examples
- Using the tabs feature, added a second StringSelect example that uses the @disnake.ui.string_select decorator inside a view rather than subclassing
@shiftinv shiftinv added t: page/section addition Pull Request to add new content to the guide. s: needs review labels Jan 5, 2023
Copy link
Member

@shiftinv shiftinv left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for adding this!
Got a couple (mostly tiny) nits and adjustments; sorry for the amount of comments c:

dlchamp and others added 10 commits January 5, 2023 17:07
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
This is cleaner

Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Ah.  The extra */} must have been added from running the pre-commit.  I didn't even notice

Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
@shiftinv shiftinv changed the base branch from dev to main January 8, 2023 14:55
dlchamp and others added 13 commits January 14, 2023 15:54
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
Altered example to disable each of the view's components by looping over them rather than using indexing
@dlchamp
Copy link
Contributor Author

dlchamp commented Jan 14, 2023

I'm not even sure if I'm doing this correctly. Thanks for bearing with me as I'm learning git/github. Mostly only ever deal with my own repos so I haven't been too worried about how certain things work here.

Copy link
Member

@shiftinv shiftinv left a comment

Choose a reason for hiding this comment

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

I'm not even sure if I'm doing this correctly. Thanks for bearing with me as I'm learning git/github. Mostly only ever deal with my own repos so I haven't been too worried about how certain things work here.

No worries, it's all good c:
For what it's worth, you can batch commit suggestions on GitHub which reduces noise a bit, but it's not strictly required as the commits all get squashed into one when merging the PR anyway.

dlchamp and others added 3 commits January 15, 2023 14:28
Co-authored-by: shiftinv <8530778+shiftinv@users.noreply.github.com>
Signed-off-by: DLCHAMP <36091350+dlchamp@users.noreply.github.com>
- Moved options into decorator and added comments to provide information describing how to define dynamic options when using the `string_select` decorator
- Added Note to provide detailed info on using a list or dict as other available methods to define `SelectOptions`
@abhigyantrips
Copy link
Member

@dlchamp please run pre-commit on all files once again, and commit any relevant changes.

dlchamp added 3 commits March 24, 2023 05:35
Reverted last commit to remove the built documentation files.  Sorry.
Pre-commit ran and this only includes the changes made to `select-menus.mdx`
Copy link
Member

@abhigyantrips abhigyantrips left a comment

Choose a reason for hiding this comment

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

This is a major addition to the guide, so thank you for the contribution!

The images in the guide so far have been extremely opinionated (having default role colors, using a "Disnake Bot" account, etc.) and might be changed in the future. But I approve of the example and implementation.

@abhigyantrips abhigyantrips requested a review from shiftinv March 25, 2023 11:11
Copy link
Member

@shiftinv shiftinv left a comment

Choose a reason for hiding this comment

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

Other than the one remaining comment, lgtm! Thank you for taking the time to add all of this.

Updated image to keep with current examples using "Disnake Bot", default role color, and Disnake logo as the bot icon
@dlchamp
Copy link
Contributor Author

dlchamp commented Jul 8, 2023

This is a major addition to the guide, so thank you for the contribution!

The images in the guide so far have been extremely opinionated (having default role colors, using a "Disnake Bot" account, etc.) and might be changed in the future. But I approve of the example and implementation.

Updated the image to fall inline with other examples using "Disnake Bot" with the Disnake logo and default role colors.

Copy link
Member

@abhigyantrips abhigyantrips left a 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 for the contribution. We'll start working on the merge.

@dlchamp dlchamp closed this Aug 4, 2023
@dlchamp dlchamp deleted the update-page/select-menu branch August 4, 2023 13:27
@abhigyantrips
Copy link
Member

@dlchamp do you want to withdraw this contribution? The changes had been reviewed and approved for merge already.

@dlchamp dlchamp restored the update-page/select-menu branch August 7, 2023 14:40
@dlchamp
Copy link
Contributor Author

dlchamp commented Aug 7, 2023

@dlchamp do you want to withdraw this contribution? The changes had been reviewed and approved for merge already.

No. I was looking at adding to some other parts of the guide and deleted the branch local without realizing it would delete the branch here as well. My mistake. Restored.

@shiftinv shiftinv reopened this Aug 7, 2023
@abhigyantrips abhigyantrips merged commit e3b801b into DisnakeDev:main Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

s: needs review t: page/section addition Pull Request to add new content to the guide.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Update page with examples and more information covering the various SelectMenus available within Disnake

3 participants