8000 [Notifier] document the array shape of the content option by xabbuh · Pull Request #60342 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Notifier] document the array shape of the content option #60342

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 17, 2025
Merged

[Notifier] document the array shape of the content option #60342

merged 1 commit into from
May 17, 2025
+32 −0

Conversation

xabbuh
Copy link
Member
@xabbuh xabbuh commented May 4, 2025
Q A
Branch? 7.3
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #60140 (comment)
License MIT

but I wonder: is it actually correct that $content is an array, according to https://www.w3.org/TR/activitystreams-vocabulary/#dfn-content I would have guessed that it needs to be a string or (if it was an array), we would have to call it contentMap in the payload instead

@xabbuh xabbuh requested a review from OskarStark as a code owner May 4, 2025 20:03
@carsonbot carsonbot added this to the 7.3 milestone 8000 May 4, 2025
@carsonbot carsonbot changed the title [Notifier] document the array shape of the content option [Notifier] document the array shape of the content option May 4, 2025
@xabbuh xabbuh added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label May 5, 2025
@stof
Copy link
Member
stof commented May 5, 2025

AFAIK, this content option has nothing to do with the activitystreams specification of the W3C. It has to do with symfony/ux#1853

@stof
Copy link
Member
stof commented May 5, 2025

And this means that the type added in this PR is the wrong one.

@xabbuh
Copy link
Member Author
xabbuh commented May 8, 2025

I have no idea how to read the code of the linked pull request in regard to the changes we want to do here. What would be the right array shape to use?

@stof
Copy link
Member
stof commented May 12, 2025

Looking at the typescript types, they actually refer to the options supported by the Notification constructor of the web platform: https://developer.mozilla.org/en-US/docs/Web/API/Notification/Notification#options

@xabbuh xabbuh force-pushed the pr-60140 branch 2 times, most recently from 9cba2a0 to 2b9721f Compare May 12, 2025 09:19
@xabbuh xabbuh requested a review from stof May 16, 2025 07:06
@fabpot
Copy link
Member
fabpot commented May 17, 2025

Thank you @xabbuh.

@fabpot fabpot merged commit 94f4d7a into symfony:7.3 May 17, 2025
16 of 17 checks passed
@xabbuh xabbuh deleted the pr-60140 branch May 17, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexandre-daubois alexandre-daubois alexandre-daubois left review comments

@fabpot fabpot fabpot approved these changes

@OskarStark OskarStark Awaiting requested review from OskarStark OskarStark is a code owner

@stof stof Awaiting requested review from stof

Assignees
No one assigned
Labels
Notifier ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
5 participants
@xabbuh @stof @fabpot @alexandre-daubois @carsonbot
0