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

Add weight to links spec #26233

Merged
merged 1 commit into from May 17, 2023
Merged

Add weight to links spec #26233

merged 1 commit into from May 17, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 16, 2023

Overview

This allows a weight to be set & for it to alter the order.

Before

Very cumbersome to alter link order in an extension

After

Can set 'weight'

Technical Details

It is a bit hard to know whether to add weights to existing ones as it would be a lot to do all of them - but I did hear a specific request (can't recall who from) to have Delete last to reduce mis-clicks and with the View & Edit declared in the same place I felt I should add them with low values for some sort of consistency

This would mess with people who have implemented weight in a hook - but probably any change is low impact & an easy-ish fix

Note that I will update docs once agreed

Comments

https://lab.civicrm.org/documentation/docs/dev/-/blob/master/docs/hooks/changes.md

@civibot
Copy link

civibot bot commented May 16, 2023

(Standard links)

@colemanw
Copy link
Member

This would mess with people who have implemented weight in a hook - but probably any change is low impact & an easy-ish fix

If weight isn't supported, why would anyone have done that in an extension?

@eileenmcnaughton
Copy link
Contributor Author

@colemanw no I mean they might have mimiced it with a bunch of array_shifting

This allows a weight to be set & for it to alter the order.

It is a bit hard to know whether to add weights to existing ones as
it would be a lot to do all of them - but I did hear a specific request
(can't recall who from) to have Delete last to reduce mis-clicks
and with the View & Edit declared in the same place
I felt I should add them with low values for some sort of consistency

This would mess with people who have implemented weight in a hook - but
probably any change is low impact & an easy-ish fix

Note that I will update docs once agreed
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 that higher level php on Ci is catching a bunch of type issues that don't show up locally - ie 1) it's good to run Ci on php 8 & 2) if we can sort out the style issue here #26002 we can start adding strict to our unit tests so people will hit the same errors locally regardless of php version

@eileenmcnaughton
Copy link
Contributor Author

@colemanw once this is merged I'm gonna add a bunch of weights - I think it would be nicer for devs if they all have weights - but I might have some different test issues so will get this merged first

@colemanw
Copy link
Member

Ok, I think this is a positive move.

@colemanw colemanw merged commit 12c48f7 into civicrm:master May 17, 2023
3 checks passed
@colemanw colemanw deleted the weight branch May 17, 2023 00:24
wmfgerrit pushed a commit to wikimedia/wikimedia-fundraising-crm that referenced this pull request May 22, 2023
Also - I didn't port it but I proposed a new key on weight
civicrm/civicrm-core#26233
so I set a value here for if it merges

Change-Id: If66663f9f342be11d0e7ef07199f363bec92ecf1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants