Skip to content

DiscordWebhooks #1355

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

Open
wants to merge 41 commits into
base: main
Choose a base branch
from

Conversation

JoanFo1456
Copy link
Contributor

After repairing some issues i had with my git, i suppose now is ready to work. (Did a pr before)
Some files probably have disappeared on another ones with the new changes recently, same some files probably need to be on another part, after all, this was a update following main but some files weren't removed at all...
I will make changes on it, making it less "draft" if this is accepted, only change that needs to be done at least i think is when saving to reload filament with the new changes.

@JoanFo1456
Copy link
Contributor Author

image
Lance asked for a preview, this is an example i made, i'm only using the embed editor, but also works with messages only, variables on the payload can be used on the preview and discord.
When a variable like name doesn't exist on the view, it uses Faker to show a name.

@JoanFo1456 JoanFo1456 force-pushed the feature/DiscordWebhook branch from f28cb83 to 3286514 Compare May 9, 2025 19:12
Copy link
Member

@lancepioch lancepioch 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 the hard work so far, keep it up!

@Boy132 Boy132 linked an issue May 12, 2025 that may be closed by this pull request
@JoanFo1456
Copy link
Contributor Author

Lance, if you get the chance let me know if everything is good or not, my suggestion here is adding once again faker to show example values on the embed, instead of giving the values directly.

@JoanFo1456 JoanFo1456 requested a review from lancepioch May 13, 2025 21:38
@pelican-dev pelican-dev deleted a comment from JoanFo1456 May 14, 2025
@JoanFo1456
Copy link
Contributor Author

I have used a phpstan ignore next line on DiscordPreview, since i didn't quite found a way, i'm open to suggestions...

@JoanFo1456 JoanFo1456 requested a review from lancepioch May 15, 2025 21:13
Copy link
Member

@lancepioch lancepioch left a comment

Choose a reason for hiding this comment

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

Super close, thanks

@JoanFo1456 JoanFo1456 requested a review from lancepioch May 26, 2025 22:24
@JoanFo1456
Copy link
Contributor Author

Still can't be merged, since the change you asked headers stopped being sent, i'm looking at it.

@JoanFo1456
Copy link
Contributor Author

Now you can take a look and everything...

@JoanFo1456 JoanFo1456 requested a review from lancepioch May 27, 2025 18:10
@JoanFo1456
Copy link
Contributor Author

Everything seems good to be merged, so if you guys want, go ahead.

])
->afterStateHydrated(function ($state, Set $set, Get $get) {
if ($state === WebhookType::Discord->value) {
AlertBanner::make()
Copy link
Member

Choose a reason for hiding this comment

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

Set id for alert banner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this, seems that something doesn't like it, since the banner never gets shown (the code is running), so I don't actually know what happened.

Copy link
Member

Choose a reason for hiding this comment

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

That's a known issue atm, "dynamic" alert banners (aka banners created after the page is loaded) need a page refresh to show.

The main purpose of the id is to prevent the same banner showing multiple times.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, okay. Also was a flag of mine, once I repaired it I saw that, that is sad though, that you need to reload to see the actual banner, maybe if I make the save now button restart page if it detects a change of type?

@JoanFo1456 JoanFo1456 requested a review from Boy132 June 10, 2025 14:02
@JoanFo1456
Copy link
Contributor Author

You guys think is good enough to be on the panel? Or should I make some other changes

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.

Webhooks in discord format
5 participants