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

feat(Navigation): emit dedicated event for loading additional entries #49904

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Dec 17, 2024

Summary

Currently, apps that add custom/dynamic entries have to listen to the BeforeTemplateRenderedEvent when they intend to add navigation entries. This is already rather a hack and relies on template rendering.

But there is the endpoint of the NavigationController in core that offers to get the current data of the app navigation. The app management is making use of it, and Tables would like to do so too. But in this code path, the above mentioned Event is not being fired. Hence, fire a dedicated event.

This is a grey zone between a feature and a bug. For the other use case in tables we can work around it with an own endpoint, for the app management this can be considered a bug, and solving it would require the change to be backported. It would not be breaking. Since it is nowhere critical I am fine having it for 31 only.

Screenrecording of the problem:

Screencast_20241217_202733.webm

With this change and modified tables app:

Screencast_20241217_202922.webm

Checklist

@blizzz blizzz added enhancement 3. to review Waiting for reviews labels Dec 17, 2024
@blizzz blizzz added this to the Nextcloud 31 milestone Dec 17, 2024
@blizzz blizzz requested review from provokateurin, a team, nfebe and skjnldsv and removed request for a team December 17, 2024 19:33
@blizzz blizzz force-pushed the enh/noid/navigationentryevent branch from d35ece4 to 71a1fbb Compare December 17, 2024 21:01
@blizzz blizzz force-pushed the enh/noid/navigationentryevent branch 2 times, most recently from 3965b0a to b48f1f0 Compare December 18, 2024 11:43
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz force-pushed the enh/noid/navigationentryevent branch from b48f1f0 to 13fefc4 Compare December 18, 2024 11:50
@blizzz
Copy link
Member Author

blizzz commented Dec 18, 2024

Adjusted and extended test, fixed another instantiation (in file`s App.php) and updated autoloader info.

I think the failing test is unrelated, but don't really know what live photo is.

@provokateurin
Copy link
Member

Live photo also fails in other PRs atm, so unrelated indeed

@blizzz blizzz merged commit d2923aa into master Dec 18, 2024
186 of 188 checks passed
@blizzz blizzz deleted the enh/noid/navigationentryevent branch December 18, 2024 18:41
@skjnldsv skjnldsv mentioned this pull request Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants