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

Reload Twinklefluff interface when diff view changes #407

Merged
merged 4 commits into from
Mar 21, 2019
Merged

Reload Twinklefluff interface when diff view changes #407

merged 4 commits into from
Mar 21, 2019

Conversation

WMDE-Fisch
Copy link
Contributor

This patch will hook Twinklefluff initialization to the mw.hook
responsible for pages containing a diff. Hooking is only done when
the methods for diff pages are loaded.

Now for example the RevisionSlider extension that fires that hook
will re-initiate the Twinkle links added to the page when moving
through the revisions.

This should fix #360

The hook gets fired everytime a diff page loads / reloads. To
avoid having the diff links twice the original method got re-
factored and the part addressing diff pages reacts only to the
hook now.

Further modules that might need to be re-initiated by hook
could be added in a follow up.

This patch will hook Twinklefluff initialization to the mw.hook
responsible for pages containing a diff. Hooking is only done when
the methods for diff pages are loaded.

Now for example the RevisionSlider extension that fires that hook
will re-initiate the Twinkle links added to the page when moving
through the revisions.

This should fix #360

The hook gets fired everytime a diff page loads / reloads. To
avoid having the diff links twice the original method got re-
factored and the part addressing diff pages reacts only to the
hook now.

Further modules that might need to be re-initiated by hook
could be added in a follow up.
@WMDE-Fisch
Copy link
Contributor Author

The diff is a bit ugly. But I separated what looked like stuff affecting diff pages from stuff affecting other pages in Twinklefluff. Just put it into own methods and then applied the hooking on the part ( I assume ) loaded for diff pages.

@WMDE-Fisch
Copy link
Contributor Author

poke
@azatoth or anyone :-)

@MusikAnimal
Copy link
Collaborator

Here's the diff with whitespace removed, makes it a bit easier to review: https://github.com/azatoth/twinkle/pull/407/files?diff=unified&w=1

I'm going to try to test this today.

modules/twinklefluff.js Outdated Show resolved Hide resolved
@Amorymeltzer Amorymeltzer self-assigned this Mar 14, 2019
@Amorymeltzer
Copy link
Collaborator

I want to get #511 merged first, since I think that will create some nasty conflicts, but once that's done I'll merge changes into this and throw it up on test.wiki; my initial tests looked good!

@Amorymeltzer
Copy link
Collaborator

@WMDE-Fisch Finally got around to this — I've dealt with the merge conflicts here — but as I can't commit to your branch, if you could commit the two suggestions I've made (one necessary, one cosmetic) this looks good to go AFAICT.

Implementing suggestions by Amorymeltzer,

Co-Authored-By: WMDE-Fisch <christoph.jauera@wikimedia.de>
@WMDE-Fisch
Copy link
Contributor Author

@WMDE-Fisch Finally got around to this — I've dealt with the merge conflicts here — but as I can't commit to your branch, if you could commit the two suggestions I've made (one necessary, one cosmetic) this looks good to go AFAICT.

Thanks for the suggestions, I haven't looked into this for some time now and do not really have the time to test this atm. But if you're confident that its working feel free to merge.

Co-Authored-By: WMDE-Fisch <christoph.jauera@wikimedia.de>
@Amorymeltzer Amorymeltzer dismissed atlight’s stale review March 21, 2019 14:12

Committed changes solve the issue, seem to work

@Amorymeltzer Amorymeltzer merged commit d5f087c into wikimedia-gadgets:master Mar 21, 2019
@Amorymeltzer Amorymeltzer removed their assignment Apr 1, 2019
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request May 10, 2019
…iff content hidden

Fixes wikimedia-gadgets#627.  If a user has "Do not show page content below diffs" selected in Special:Preferences, `wgRevisionId` will be set to 0 on diffs, meaning no links would be shown on diffs.  Comparing `wgRevisionId` to `wgDiffNewId` from wikimedia-gadgets#563 appears to be a holdover from some (local) attempts to handle merge conflicts between wikimedia-gadgets#511 and wikimedia-gadgets#407, when I initially had this structured differently, but that should now handled in the init, so this was excessive.
Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Jan 29, 2020
`fluff` is already processing diffs, so this saves `warn` from checking anything on diffs.  Plus, this takes advantage of the already in-place handling of the revision slider hook (wikimedia-gadgets#407) so that these `extraParams` are also added to user talk links when the revision slider is updated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Twinkle JS does not reload after RevisionSlider is used to switch diffs
4 participants