Provide local task for editing current dashboard layout

This patch applies on top of ✨ Show dashboards as local tasks Fixed , so postponing on that one.

Oops I renamed the route last minute, changing local tasks too.

I think visually it's less weird with a local action instead of local tasks, which produces all kind of weird behaviors as we are using as a local task a route that has already its own local tasks, so we see duplicated tabs etc (even when trying a secondary level). Ideally we would revisit this when/if https://www.drupal.org/project/ideas/issues/3325034 🌱 Providing additional methods of navigating the admin interface Active materializes.

I let both implementations for comparison at the tugboats at:

local task: https://github.com/penyaskito/dashboard-initiative/pull/10
local action: https://github.com/penyaskito/dashboard-initiative/pull/17

Hi,

Thank you for your great work here. Tested both Tugboat environments, and I would go for the Local Action based approach.

It looks cleaner to me. The "Edit Layout" local task at the same level can be confusing and it requires the extra redirect you mentioned.

In case we want to follow the local action path, we might try to add the parent_id property to show it in a second level, like in /admin/structure/block.

Both options would require an intermediate redirect for the "default" dashboard. And you can't specify a parent_id (or a base route) with arguments, or I didn't found a way, so the local tasks ended up being a mess (even more with a second level).

Maybe you want to give it a try?

Agree that nested local tasks can be problematic, so let's put them apart for now.

Attaching patch following a custom local task class approach instead of the intermediate controller. This approach could be useful as well if we need to move to local tasks in the future.

1. +++ b/src/DashboardManager.php
   @@ -0,0 +1,66 @@
   +  /**
   +   * The current user account.
   +   *
   +   * @var \Drupal\Core\Session\AccountInterface
   +   */
   +  protected $currentUser;
   +
   +  /**
   +   * The entity type manager.
   +   *
   +   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   +   */
   +  protected $entityTypeManager;
   ...
   +  public function __construct(AccountInterface $current_user, EntityTypeManagerInterface $entity_type_manager) {
   

   - public function __construct(AccountInterface $current_user, EntityTypeManagerInterface $entity_type_manager) {
   +public function __construct(protected AccountInterface $current_user, protected EntityTypeManagerInterface $entity_type_manager) {
   

2. +++ b/src/DashboardManager.php
   @@ -0,0 +1,66 @@
   +   * @return \Drupal\dashboard\DashboardInterface|null
   +   *   The dashboard to be used if available, NULL otherwise.
   ...
   +  public function getDefaultDashboard(AccountInterface $account = NULL) {
   

   could return type ?DashboardInterface

3. +++ b/src/Plugin/Menu/LocalAction/DashboardLocalAction.php
   @@ -0,0 +1,73 @@
   +   * The dashboard manager.
   +   *
   +   * @var \Drupal\dashboard\DashboardManager
   +   */
   +  protected $dashboardManager;
   ...
   +    DashboardManager $dashboard_manager
   

   could be moved to protected DashboardManager $dashboard_manager in constructor

Back to Needs Review once #3348617 is already merged.

Thank you for your review @andypost. I'm fine these PHP 8.1 style code changes, but I don't know what's the policy about it in Drupal core right now. Reviewed some modules and apparently they're not not following it yet. In any case I would do it at once for the whole module at some point instead of following it in specific places and other don't.

Core started to adopt constructor promotions already, it helps to minimize boilerplate for properties.

I agree with @andypost here. Better start doing it already than accumulating more tech debt.
I've tested this and it looks great. @plopesc Happy to merge this as is, and we do a quick round of constructor promotions in a follow-up?

Sounds like a plan!

Thank you for checking it.

Created https://www.drupal.org/project/3327580/issues/3367934 📌 Use constructor promotions Fixed

Committing this now after checking in https://github.com/penyaskito/dashboard-initiative/pull/19 that works as expected and tests pass.

Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.