Problem/Motivation

Currently in the core/modules/navigation/src/Attribute/TopBarItem.php attribute there is no weight property which could be used by the plugin manager to control the order of the items in each top bar regions.

Proposed resolution

- Add a weight property to Top Bar items.
- Make the plugin manager sort on this weight.

Or instead of weight, add a priority logic.

Remaining tasks

Agree on solution.

API changes

New attribute property.

Release notes snippet

CommentFileSizeAuthor
#9 3538221-nr-bot__xu5zjsx.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3538221

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:

About issue forks

Comments

grimreaper created an issue. See original summary.

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

riyas_nr opened merge request !12885

riyas_nr’s picture

riyas_nr commented

Implemented weight definition for TopBarItem plugins.
Sorted plugins based on weight in the plugin manager and added test coverage to verify correct rendering order.

riyas_nr’s picture

riyas_nr commented
Status: Active » Needs review
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs review » Needs work

Thanks @riyas_nr for the MR.

Looks good!

I put a review comment and pipeline needs to pass.

riyas_nr’s picture

riyas_nr commented
Status: Needs work » Needs review

Thanks @grimreaper for the review.
I've moved the sorting logic to the getDefinitions() override as suggested.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs review » Reviewed & tested by the community

I have tested the MR with https://git.drupalcode.org/project/sobki_profile_bootstrap/-/blob/11.0.x..., locally setting a weight to control the order to test.

It works as expected!

Also given a look at the MR, ok for me.

Now needs maintainers review.

Thanks @riyas_nr !

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3538221-nr-bot__xu5zjsx.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs work » Reviewed & tested by the community

MR rebased.

Unrelated build test failure, back to RTBC.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Wasn't 100% sure about using weight here, elsewhere we're trying to move to before/after - however we already use weight in a lot of different places, and we only expect a handful of top bar items (especially shown at the same time), so I think this is fine.

Committed/pushed to 11.x, thanks!

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

  • catch committed 23a802d7 on 11.x 
    Issue #3538221 by grimreaper, riyas_nr: Be able to control the order of...

catch closed merge request !12885

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Thanks for the merge!

Status: Fixed » Closed (fixed)

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