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.

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

feat: frontend Model extender #3646

Merged
merged 19 commits into from
Feb 8, 2023
Merged

feat: frontend Model extender #3646

merged 19 commits into from
Feb 8, 2023

Conversation

SychO9
Copy link
Member

@SychO9 SychO9 commented Sep 16, 2022 •
edited
Loading

Relies on #3645

Changes proposed in this pull request:
Follow up to the linked PR. Adds a Model frontend extender and makes use of it in bundled extensions.

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

@SychO9 SychO9 requested a review from a team as a code owner September 16, 2022 17:43
@SychO9 SychO9 self-assigned this Sep 17, 2022
luceos
luceos approved these changes Oct 1, 2022
View reviewed changes
Copy link
Member

@luceos luceos left a comment

Choose a reason for hiding this comment

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

I love how much simpler the extensibility becomes in this way 👍

@SychO9
chore: naming
e26283a 
Signed-off-by: Sami Mazouz <[email protected]>
askvortsov1
askvortsov1 reviewed Oct 3, 2022
View reviewed changes
Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Do we have any way to declare stubs / bindings so that these supplemental attributes are available through typing?

I know it would need to be done explicitly, but feels like it should be possible.

extensions/flags/js/src/forum/extend.ts Outdated Show resolved Hide resolved
extensions/suspend/js/src/forum/extend.ts Outdated Show resolved Hide resolved
@davwheat
Copy link
Member

davwheat commented Oct 3, 2022

Do we have any way to declare stubs / bindings so that these supplemental attributes are available through typing?

I think we've been using a shims.d.ts file for adding typings for a while, now.

E.g...

@SychO9
Copy link
Member Author

SychO9 commented Oct 3, 2022 •
edited
Loading

⬆️ typing-wise, extender or not, we still have to declare additional typings the same way, though let me check further

@SychO9
Merge branch 'main' into sm/frontend-extender
c4691e5 
# Conflicts:
#	extensions/mentions/js/src/forum/index.js
#	extensions/package-manager/js/src/admin/index.tsx
@SychO9
Merge branch 'sm/frontend-extender' into sm/frontend-extender-model
993f765
@SychO9
chore: merge conflicts
bf73369 
Signed-off-by: Sami Mazouz <[email protected]>
@SychO9
Merge branch 'main' into sm/frontend-extender
e4f2b87
@SychO9
chore: unused import
7b83bdf 
Signed-off-by: Sami Mazouz <[email protected]>
@SychO9
Merge branch 'sm/frontend-extender' into sm/frontend-extender-model
aa61e68 
# Conflicts:
#	extensions/tags/js/src/admin/index.ts
#	extensions/tags/js/src/forum/index.ts
@SychO9 SychO9 added the javascript Pull requests that update Javascript code label Jan 15, 2023
askvortsov1
askvortsov1 requested changes Jan 17, 2023
View reviewed changes
extensions/flags/js/src/forum/extend.ts Outdated Show resolved Hide resolved
extensions/flags/js/src/forum/extend.ts Outdated Show resolved Hide resolved
extensions/tags/js/src/forum/extend.ts Outdated Show resolved Hide resolved
Base automatically changed from sm/frontend-extender to main January 17, 2023 18:10
@SychO9
Merge branch 'main' into sm/frontend-extender-model
12034e1 
# Conflicts:
#	extensions/flags/js/src/forum/extend.ts
#	extensions/flags/js/src/forum/index.ts
#	extensions/likes/js/src/forum/extend.ts
#	extensions/lock/js/src/forum/extend.ts
#	extensions/lock/js/src/forum/index.js
#	extensions/mentions/js/src/forum/extend.ts
#	extensions/mentions/js/src/forum/index.js
#	extensions/sticky/js/src/forum/extend.ts
#	extensions/sticky/js/src/forum/index.js
#	extensions/subscriptions/js/src/forum/extend.ts
#	extensions/subscriptions/js/src/forum/index.js
#	extensions/tags/js/src/forum/extend.ts
#	extensions/tags/js/src/forum/index.ts
#	framework/core/js/src/common/extenders/index.ts
@SychO9
chore: multiline extenders
f6c5ab9 
Signed-off-by: Sami Mazouz <[email protected]>
@SychO9 SychO9 requested a review from askvortsov1 January 21, 2023 15:08
@SychO9
feat: add Store extender
56c5149 
Signed-off-by: Sami Mazouz <[email protected]>
@SychO9 SychO9 force-pushed the sm/frontend-extender-model branch from 520c228 to 56c5149 Compare February 8, 2023 20:03
@SychO9 SychO9 merged commit 47b670a into main Feb 8, 2023
@SychO9 SychO9 deleted the sm/frontend-extender-model branch February 8, 2023 20:13
@SychO9 SychO9 added this to the 1.7 milestone Feb 8, 2023
@SychO9 SychO9 mentioned this pull request Feb 12, 2023
Merged
@luceos luceos mentioned this pull request Mar 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luceos luceos luceos approved these changes

@askvortsov1 askvortsov1 Awaiting requested review from askvortsov1

Assignees

@SychO9 SychO9

Labels
javascript Pull requests that update Javascript code prio/high type/feature
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
4 participants
@SychO9 @davwheat @luceos @askvortsov1