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

[Guide] Embeds guide #9708

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

[Guide] Embeds guide #9708

wants to merge 23 commits into from

Conversation

Soheab
Copy link
Contributor

@Soheab Soheab commented Jan 13, 2024

Summary

(This is continued from NCPlayz's PR at #7498)

Embeds guide.

Checklist

  • This PR is not a code change (e.g. documentation, README, ...)

@Soheab Soheab changed the title [Guide][WIP] Embeds guide [Guide] Embeds guide Jan 13, 2024
Copy link
Contributor

@ika2kki ika2kki left a comment

Choose a reason for hiding this comment

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

review~

some general feedback pertaining to the whole guide:
overall, i think the structure and flow of the guide could improve a lot. i dont like how each field is introduced individually. i feel the guide should go over the basics:

  • passing fields to the constructor
  • alternatively using the setters
  • set_x and add_field
  • nuances like attaching a file, library casting to str
  • a restrictions and limits section
  • recipes with some cool looking embeds and a tip about ZWS
    and then leave room for readers to experiment with other fields themselves.

i feel theres also way too many screenshots

i wanted to touch on a few other stuff as well but im a bit tired for now.

thanks for your work, feel free to ignore any advice etc. etc.

docs/guide/topics/embeds.rst Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@IAmTomahawkx IAmTomahawkx left a comment

Choose a reason for hiding this comment

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

Some nitpicks on layout and english

docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@AbstractUmbra AbstractUmbra left a comment

Choose a reason for hiding this comment

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

Nicely done and well thought out, just one (or two) queries.

docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
docs/guide/topics/embeds.rst Outdated Show resolved Hide resolved
Copy link

@No767 No767 left a comment

Choose a reason for hiding this comment

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

From a first glance, there isn't much that I can point out. Nicely done.

~~~~~~~~~~~~~~~~~

Each field has its own character limit. Unfortunately, listing all the limits here would quickly become
outdated, but you can find them in the :ddocs:`Discord API documentation<resources/channel#embed-object>`.
Copy link

Choose a reason for hiding this comment

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

Suggested change
outdated, but you can find them in the :ddocs:`Discord API documentation<resources/channel#embed-object>`.
outdated, but you can find them in the :ddocs:`Discord API documentation<resources/channel#embed-object-embed-limits>`.

The link supplied isn't the correct one as you would need to scroll down further, but this one should be the correct one: https://discord.com/developers/docs/resources/channel#embed-object-embed-limits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

6 participants