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

Quick fix on summary actions conflict #19067

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Nov 28, 2020

Overview

Per eileenmcnaughton/org.wikimedia.contacteditor#8 &
mattwire/uk.co.mjwconsult.checksum#1

menu items are indexed by weight, resulting in a conflict between 2 items with the same weight -
this is just a quick & dirty fix to ensure we don't overwrite one menu item with another

Before

If 2 extensions add menu links with the same weight only the second one processed is displayed as the values are keyed by weight & the first is overwritten by the second

After

The weight is bumped up if already in use, avoiding some being lost

Technical Details

Comments

@civibot
Copy link

civibot bot commented Nov 28, 2020

(Standard links)

@colemanw
Copy link
Member

@eileenmcnaughton what about changing the if to while? That way it will keep incrementing until it gets a unique value.

Per eileenmcnaughton/org.wikimedia.contacteditor#8 &
mattwire/uk.co.mjwconsult.checksum#1

menu items are indexed by weight, resulting in a conflict between 2 items with the same weight -
this is just a quick & dirty fix to ensure we don't overwrite one menu item with another
@eileenmcnaughton
Copy link
Contributor Author

@colemanw done - also fixed enotices hurting tests

@eileenmcnaughton
Copy link
Contributor Author

@colemanw this should be mergeable

@colemanw colemanw merged commit a11f15b into civicrm:master Dec 3, 2020
@colemanw colemanw deleted the weight branch December 3, 2020 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants