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

Files additionalscripts to real event #16641

Merged
merged 2 commits into from
Sep 24, 2019
Merged

Files additionalscripts to real event #16641

merged 2 commits into from
Sep 24, 2019

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Aug 2, 2019

  • New additionalscripts event
  • Legacy handler
  • Moved comments as a POC

This allows to write nice tiny classes that just do what they need instead of having code and connetion mixed.

@rullzer rullzer added enhancement 3. to review Waiting for reviews labels Aug 2, 2019
@rullzer rullzer added this to the Nextcloud 17 milestone Aug 2, 2019
@ChristophWurst
Copy link
Member

oh an conflicts :)

@rullzer rullzer force-pushed the enh/files_additionalscripts branch from e9097c2 to fb4dd15 Compare August 7, 2019 17:43
@rullzer
Copy link
Member Author

rullzer commented Aug 7, 2019

updated :)

@MorrisJobke MorrisJobke mentioned this pull request Aug 8, 2019
Merged
28 tasks
ChristophWurst
ChristophWurst requested changes Aug 8, 2019
View reviewed changes
apps/comments/lib/AppInfo/Application.php
}
);
protected function registerSidebarScripts(IEventDispatcher $dispatcher) {
$dispatcher->addServiceListener(LoadAdditionalScriptsEvent::class, LoadAdditionalScripts::class);
Copy link
Member

Choose a reason for hiding this comment

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

for one-liners you could also use the addListener and pass a closure. but fine by me to have it in a class :)

apps/files/lib/Listener/LegacyLoadAdditionalScriptsAdapter.php Outdated Show resolved Hide resolved
apps/files/lib/Listener/LegacyLoadAdditionalScriptsAdapter.php Outdated Show resolved Hide resolved
@rullzer
Copy link
Member Author

rullzer commented Aug 8, 2019

Lets do this for 18 (early) as we need other events as well and more functionality.

@rullzer rullzer modified the milestones: Nextcloud 17, Nextcloud 18 Aug 8, 2019
@nickvergessen
Copy link
Member

I would also prefer to merge the 2 event dispatchers if easily possible (one wrapping the other or somthing)

@ChristophWurst
Copy link
Member

I would also prefer to merge the 2 event dispatchers if easily possible (one wrapping the other or somthing)

That is already the case using composition and delegation. There is only one instance of a dispatcher, but two interfaces for it.

@kesselb
Copy link
Contributor

kesselb commented Aug 26, 2019

image

@rullzer
Copy link
Member Author

rullzer commented Sep 9, 2019

@nickvergessen please have another look. cleaner now

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 15, 2019
@rullzer rullzer force-pushed the enh/files_additionalscripts branch from 051ae3e to 2cc9447 Compare September 15, 2019 07:53
@rullzer rullzer added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Sep 15, 2019
@rullzer
Copy link
Member Author

rullzer commented Sep 15, 2019

Recent new eventdispatcher edits broke this. I'll have to take another look.

@rullzer
Add new LoadAdditionalScriptsEvent
606b4d5 
This adds a new event that is in the new style of event dispatching.
This should allow more lazy loading and better sepeartion of concerns

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Move over comments to the new event
5694a04 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the enh/files_additionalscripts branch from f5a27b8 to 5694a04 Compare September 23, 2019 08:10
@rullzer rullzer added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Sep 23, 2019
@nickvergessen
Copy link
Member

So, ready to merge?

@rullzer rullzer merged commit 88b2d8f into master Sep 24, 2019
@rullzer rullzer deleted the enh/files_additionalscripts branch September 24, 2019 19:56
MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke
Document existing events
da43209 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobke MorrisJobke mentioned this pull request Aug 10, 2020
Merged
backportbot-nextcloud bot pushed a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke @backportbot
Document existing events
6ca7040 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke
Document existing events
7e78e7e 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
MorrisJobke added a commit to nextcloud/documentation that referenced this pull request Aug 10, 2020
@MorrisJobke
Document existing events
6da5114 
* LoadAdditionalScripts (@rullzer) - nextcloud/server#16641
* LoadViewerEvent (@skjnldsv) - nextcloud/viewer#271
* RegisterDirectEditorEvent (@juliushaertl) - nextcloud/server#17625
* typed events for files scanner (@ChristophWurst) - nextcloud/server#18351
* typed events for group mangement (@ChristophWurst) - nextcloud/server#18350
* AddContentSecurityPolicyEvent (@rullzer) - nextcloud/server#15730
* UserLiveStatusEvent (@georgehrke) - nextcloud/server#21186
* password_policy events (@ChristophWurst) - nextcloud/server#18019
* AddFeaturePolicyEvent (@rullzer) - nextcloud/server#16613
* ShareCreatedEvent (@rullzer) - nextcloud/server#18384
* LoadSettingsScriptsEvent (@blizzz) - nextcloud/server#21475
* flow events (@rullzer) - nextcloud/server#18535

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@icewind1991 icewind1991 icewind1991 approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement
Projects
None yet
Milestone
Nextcloud 18.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@rullzer @ChristophWurst @nickvergessen @kesselb @icewind1991