Add complaint link between 'about' and 'disclaimers'#487
Conversation
note:
|
tarrow
left a comment
tarrow
left a comment
There was a problem hiding this comment.
Looks like this is going in the right direction!
I think there's some work to do both on the placement and the logic of the placement.
Also if you add Bug: T397906 to the bottom of the commit message then this should get nicely tagged on our workboard.
| if ( $key === 'places' ) { | ||
| $aboutIndex = array_search('about', array_keys($footerLinks)); // Find the position of 'about' | ||
| if ($aboutIndex === false) { | ||
| $footerLinks['report'] = Html::element( |
There was a problem hiding this comment.
Great choice here to use Html::element and not Html::rawElement like the manual suggests. This might prevent us failing to escape the content if we change the content in future.
| // add the link to the report illegal content page | ||
| $wgHooks['SkinAddFooterLinks'][] = function ( $skin, $key, &$footerLinks ) { | ||
| if ( $key === 'places' ) { | ||
| $aboutIndex = array_search('about', array_keys($footerLinks)); // Find the position of 'about' |
There was a problem hiding this comment.
Let's just check I understand this correctly:
- you want to handle the two cases here where there is is or isn't an about link?
- was that because you found places where there sometimes there isn't one?
| 'Report illegal content' | ||
| ); | ||
| } else { | ||
| $footerLinks = array_slice($footerLinks, 0, $aboutIndex + 1, true) |
There was a problem hiding this comment.
For me this logic is a little hard to follow but I think I got it after a while but I'm worried I missed something here; sticking these two array_slices together with the new array in the middle is a bit hard to parse. I think it's likely we've made a mistake here.
Do you think you could make it clearer by (for example by using array_splice?). You can see it in this stackoverflow answer and in the php docs. You'd end up setting the $length param to be 0 since we don't want to remove any elements.
Indeed, for me this actually did not (locally with skaffold) get placed between allow and disclaimers but one element later (after disclaimers). See this screenshot:
|
Closed in favor of #488 |
3 participants
Bug: T397906