Page MenuHomePhabricator

Add placeholder for title tags to the moved indicators
Closed, ResolvedPublic

Description

To allow localized title tags for the moved paragraph indicators we need to introduce a placholder set in the C++ lib to replace it later on the phpside. The replace then would be done similar to the replacement for the localized line numbering.

Suggestion:
Add a data attribute like data-title-tag="new" and data-title-tag="old" in the C++ output. This then will be replaced by the corresponding localized title="Click to jump to new location" and title="Click to jump to old location" in the moved indicator tags by PHP.

After the short discussion in https://phabricator.wikimedia.org/T180602#3771139 we decided on using the classes as indicators where to hook in the titles. The additional placeholder tags will be removed again then...

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 15 2017, 2:17 PM
WMDE-Fisch removed WMDE-Fisch as the assignee of this task.Nov 15 2017, 2:27 PM
WMDE-Fisch moved this task from Proposed to Currently in sprint on the WMDE-QWERTY-Team board.

Change 391582 had a related patch set uploaded (by Jkroll; owner: Jkroll):
[mediawiki/php/wikidiff2@master] Add placeholder for title tags to the moved indicators

https://gerrit.wikimedia.org/r/391582

WMDE-Fisch assigned this task to jkroll.Nov 17 2017, 8:32 AM
WMDE-Fisch moved this task from Sprint Backlog to Review on the WMDE-QWERTY-Sprint-2017-11-14 board.

Change 391582 merged by jenkins-bot:
[mediawiki/php/wikidiff2@master] Add placeholder for title tags to the moved indicators

https://gerrit.wikimedia.org/r/391582

@WMDE-Fisch The output already has class="mw-diff-movedpara-left" and class="mw-diff-movedpara-right". Would it make sense to use those instead? It would be nice to avoid additional attributes in the wikdiff2 output that relate to features it does not provide. The semantic information appears to be there already as far as CSS or JS selection goes. If you tried these already, then it would be good to document why that failed (for future reference).

@Krinkle yeah the info is already there form the CSS that's true and I also could have used these to sneak in the title attributes. But I thought it might be better to separate things here so and avoid bounding... sure, thinking of it, with JS I just would have don the same depending on the classes to set the title tags, and I really did that in a first attempt but got the remark, that it would be nicer to have this in PHP so it does not depend on JS. ..... hmmm :-)

Ok totally makes sense, a bit bad luck that the changes to wikidiff2 moved really quickly this time and we have the placeholder in there already - but nothing set in stone yet we will just revert the patch and I will use the classes as hooks for the title.

@Legoktm Is it possible to remove a version tag? Or do we just remove the code and tag 1.5.3?

WMDE-Fisch updated the task description. (Show Details)Nov 20 2017, 11:59 AM
WMDE-Fisch moved this task from Done to Doing on the WMDE-QWERTY-Sprint-2017-11-14 board.

@Legoktm Is it possible to remove a version tag? Or do we just remove the code and tag 1.5.3?

We can just revert and tag 1.5.3.

WMDE-Fisch closed this task as Resolved.Nov 24 2017, 1:36 PM
Tobi_WMDE_SW moved this task from Done to Demoed on the WMDE-QWERTY-Team board.Feb 20 2018, 4:52 PM