Problem/Motivation

We moved the local tasks into the "More actions" drops-down as they are. The problem is that the main action to edit the content is hidden behind a the More actions drop down.

Proposed resolution

When #3484564: Define the 3 areas the Top Bar will provide is ready, this issue is the next step: on the front-end, when the URL/page is rendering on a full viewmode of an entity (content, taxonomies,...), render the Edit button outside the More actions. See image 3.

This issue is to ONLY move the Edit button outside of the drop-down, not applying any of the other visual changes the image above is providing.

User interface changes

The Edit button will appear as the primary action outside the More actions drop-down.

Release notes snippet

Issue fork drupal-3484575

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

ckrina created an issue. See original summary.

kunalkursija’s picture

Assigned: Unassigned » kunalkursija
Status: Postponed » Active

Getting started on this one locally.
Note: #3484564 is the dependency and this issue's MR can only be raised once the parent is done.

kunalkursija’s picture

Version: 11.0.x-dev » 11.x-dev

Changing the version to 11.x-dev so that the Issue fork gets created from 11.x branch.

m4olivei’s picture

Title: [PP1] Move the Edit buttonoutside the more actions drop down » Move the Edit buttonoutside the more actions drop down

Removing the postponed title prefix as #3484564: Define the 3 areas the Top Bar will provide is merged to 11.x.

ckrina’s picture

Adding the "Drupal CMS release target" to see if we could hopefully get to it.

ckrina’s picture

Also adding Navigation stable blocker

kunalkursija’s picture

I have been working on this issue and have so far completed rendering the edit button and adding partial styles. Some more styling work is pending and then will be raising the MR for this.

kunalkursija’s picture

Looks like the MR has conflicts due to changes in Plugins, Hooks & Test files. I will have to look into this and update the MR accordingly.

kunalkursija’s picture

Status: Active » Needs review

I have resolved most of the pipeline issues except the PerformanceTest that's failing. Below are the different messages that I have been getting:

Line 75: Failed asserting that 59 is identical to 61.
Line 78: Failed asserting that 1 is identical to 2.
Line 79: Failed asserting that 27 is identical to 29.
Line 83: Failed asserting that 90360 is less than 90000.

Question: I noticed that core/tests/Drupal/Tests/PerformanceTestTrait.php calculates these values, but I'm unclear whether these numbers are updated based on specific criteria or the incoming test data. Could someone provide clarification?

sagarmohite0031’s picture

StatusFileSize
new26.69 KB
new38.66 KB

Hello,
Tested and verified on Drupal 11,
MR applied successfully.
Attaching before and after screenshots

berdir’s picture

Status: Needs review » Needs work

Left a review about the edit link idenfication.

> Question: I noticed that core/tests/Drupal/Tests/PerformanceTestTrait.php calculates these values, but I'm unclear whether these numbers are updated based on specific criteria or the incoming test data. Could someone provide clarification?

It's no so much calculated, it's just what's the current expectation. If we add more CSS, then we have to increase the expectation. What's interesting is that this *lowers* the amount of cache gets/sets. That is generally good and we can just lower the values. It would be good to understand and write down which cache operations aren't happening anymore.

anjali rathod’s picture

anjali rathod’s picture

Status: Needs work » Needs review

roshni upadhyay made their first commit to this issue’s fork.

plopesc made their first commit to this issue’s fork.

lauriii’s picture

Title: Move the Edit buttonoutside the more actions drop down » Move the Edit button outside the more actions drop down
berdir’s picture

Does it make sense to postpone this on the title issue? If I understand this correctly, that one introduces an API method to fetch the current entity for which to display the title and the same logic could be applied as to when the edit button should be displayed. I think it makes sense if those two things are in sync. We should only have an Edit button if there is a title that displays for what that button?

finnsky’s picture

Assigned: anjali rathod » Unassigned

Gonna work on it

finnsky’s picture

At the moment, the module is still experimental.
We are still waiting for the final design.
We are still waiting for the icon system.

The task description says

This issue is to ONLY move the Edit button outside of the drop-down, not applying any of the other visual changes to the image above is providing.

So there is no need to make it fully consistent with the image. It is enough to just add the markup and roughly understand how it will work afterwards.

And even more so, there is no need to add deeply nested CSS that will be impossible to clean up later without significant effort

Also, there is no need to add new icons. In particular, the Pencil was already there. Let it be different, but there will not be many icons with the same meaning when we switch to the icon API. (I hope soon)
I also took the icon of three dots from https://phosphoricons.com/ where all the other icons came from.

I also think that there is no point in adding backend tests based on visual styles, for example, the icon type, but this is not my area and I did not change it.

@plopesc could you please review backend after rebase?

plopesc’s picture

Does it make sense to postpone this on the title issue? If I understand this correctly, that one introduces an API method to fetch the current entity for which to display the title and the same logic could be applied as to when the edit button should be displayed. I think it makes sense if those two things are in sync. We should only have an Edit button if there is a title that displays for what that button?

Both issues are related but not essentially depends one on the other, I think. Here we make use of $this->navigationRenderer->hasLocalTasks() and that method has been moved to a new service in #3484600: Show entity information on the Top Bar. If we merge that one first, we will need to make some adjustments here and vice-versa. Agree that things would be simpler here if we merge that one first, though.

We could postpone this one to have a better separation of concerns.

lauriii’s picture

#3484600: Show entity information on the Top Bar has landed so I think we're good to proceed here 🚀

lauriii’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new373.44 KB

I think this is looking good. I made minor tweaks to the design to make it match the original design a bit more.

Few follow-ups that we need to open:

  1. Removing the current active local task from the more actions
  2. Showing the Edit button as the primary action when viewing a forward revision
berdir’s picture

> Showing the Edit button as the primary action when viewing a forward revision

FWIW, that should be pretty trivial, just need extend the route match to allow canonical or latest_version: (canonical|latest_version). If we also want to cover it with tests then it might get more complicated.

  • nod_ committed b8ed4811 on 11.x
    Issue #3484575 by plopesc, kunalkursija, lauriii, finnsky, anjali rathod...
nod_’s picture

Status: Reviewed & tested by the community » Fixed

the dots are way too small for me, but that's subjective, no need to hold this up longer

Committed b8ed481 and pushed to 11.x. Thanks!

berdir’s picture

Thought I commented about this yesterday, follow-up issue then: #3501662: navigation top bar edit button has strange active (?) colors

Status: Fixed » Closed (fixed)

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