-
Notifications
You must be signed in to change notification settings - Fork 12
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
Wikimania branch #12
Wikimania branch #12
Conversation
Rework how review states are handled on clientside using Less Also includes code from wikimedia#8 Bug: T134597
Add history link under page name Bug: T137909
Make diff and history links appear red if page does not exist Bug: T137980
Fix link to userpage Bug: T137382
} ); | ||
$( '.records' ).on( 'click', '.js-compare-button', function () { | ||
// pass the dataset of the element as an object to toggleComparePane | ||
toggleComparePane.call(this, this.dataset); | ||
}); | ||
} ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
}) is the standard when doing MW development. I didn't find anything in the Manual for coding conventions but you'll find examples in the code. I just rely on Bryan's code to check myself: https://github.com/wikimedia/wikimedia-iegreview/blob/master/public/js/site.js#L65
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems Bryan wasn't going after conventions there (to a tee); there's a lot of whitespace missing
d4d166a
to
be7902a
Compare
// TODO: Ideally the authentication step should be taken care of by a middleware | ||
// but I haven't found a way to make it work with AJAX requests so far | ||
$requireAuth = function () use ( $slim ) { | ||
if ( !$slim->authManager->isAuthenticated() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should move this function to the middleware array above. Line 143.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also just move it to Line 200.
be7902a
to
a1404c0
Compare
All of Leon's stuff during Wikimania. Each commit represents a specific task, except "Add ability to undo your reviews" which includes the fix for T138098, see 1062088#diff-0a3a612b6e041fccdbde3eaa6b795886R215
For the purposes of reviewing, ignore all .less files except
index.less
. 72 of the 80 files changed in this PR are just the Bootstrap Less files being put into version control. Unfortunately there is no composer package or other graceful way of importing these Less files :/