mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-27 01:02:42 +01:00
Fix issues with Differential file toggling
Summary: See D2714#15. # Give anchors an `id` equal to their `name` so JX.$ can find them. # Listen for `click` s on `<a>` s instead of `hashchange` s to make toggling a bit more correct and consistent. # Add a placeholder menu item when the file is unloaded. A previous patch by @jungejason had left the item blank in that case. # In fixing (1) I found another exception when clicking on ToC links. Since those links are `differential-load`, they try to load the file before jumping to it. However, loading destroys the node it's looking for, so if you jump to an already-loaded file JX.$ complains at you. Test Plan: Make a giant diff. Click on links and try toggling files. Reviewers: epriestley, vrana Reviewed By: vrana CC: vrana, aran, Korvin Maniphest Tasks: T370 Differential Revision: https://secure.phabricator.com/D3185
This commit is contained in:
parent
80d9a276f3
commit
dbde4b9ff2
2 changed files with 37 additions and 11 deletions
|
@ -101,6 +101,8 @@ JX.behavior('differential-dropdown-menus', function(config) {
|
|||
reveal_item.setName('Entire File Shown');
|
||||
}
|
||||
|
||||
visible_item.setDisabled(true);
|
||||
visible_item.setName("Can't Toggle Unloaded File");
|
||||
var diffs = JX.DOM.scry(JX.$(data.containerID),
|
||||
'table', 'differential-diff');
|
||||
if (diffs.length > 1) {
|
||||
|
@ -109,6 +111,7 @@ JX.behavior('differential-dropdown-menus', function(config) {
|
|||
data.containerID+'."');
|
||||
} else if (diffs.length == 1) {
|
||||
diff = diffs[0];
|
||||
visible_item.setDisabled(false);
|
||||
if (JX.Stratcom.getData(diff).hidden) {
|
||||
visible_item.setName('Expand File');
|
||||
} else {
|
||||
|
|
|
@ -33,11 +33,15 @@ JX.behavior('differential-toggle-files', function(config) {
|
|||
function(e) {
|
||||
var elt = e.getData().element;
|
||||
while (elt !== document.body) {
|
||||
if (JX.Stratcom.hasSigil(elt, 'differential-diff') &&
|
||||
JX.Stratcom.getData(elt).hidden) {
|
||||
if (JX.Stratcom.hasSigil(elt, 'differential-changeset')) {
|
||||
var diffs = JX.DOM.scry(elt, 'table', 'differential-diff');
|
||||
for (var i = 0; i < diffs.length; ++i) {
|
||||
if (JX.Stratcom.getData(diffs[i]).hidden) {
|
||||
JX.Stratcom.invoke('differential-toggle-file', null, {
|
||||
diff: [ elt ],
|
||||
diff: [ diffs[i] ],
|
||||
});
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
elt = elt.parentNode;
|
||||
|
@ -45,18 +49,37 @@ JX.behavior('differential-toggle-files', function(config) {
|
|||
});
|
||||
|
||||
JX.Stratcom.listen(
|
||||
'hashchange',
|
||||
null,
|
||||
'click',
|
||||
'tag:a',
|
||||
function(e) {
|
||||
var id = window.location.hash;
|
||||
if (!id.match(/^#/)) {
|
||||
var link = e.getNode('tag:a');
|
||||
var id = link.getAttribute('href');
|
||||
if (!id.match(/^#.+/)) {
|
||||
return;
|
||||
}
|
||||
// The target may have either a matching name or a matching id.
|
||||
var target;
|
||||
try {
|
||||
target = JX.$(id.substr(1));
|
||||
} catch(err) {
|
||||
var named = document.getElementsByName(id.substr(1));
|
||||
var matches = [];
|
||||
for (var i = 0; i < named.length; ++i) {
|
||||
if (named[i].tagName.toLowerCase() == 'a') {
|
||||
matches.push(named[i]);
|
||||
}
|
||||
}
|
||||
if (matches.length == 1) {
|
||||
target = matches[0];
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
}
|
||||
JX.Stratcom.invoke('differential-toggle-file-request', null, {
|
||||
element: JX.$(id.substr(1)),
|
||||
element: target,
|
||||
});
|
||||
// This event is processed after the hash has changed, so it doesn't
|
||||
// automatically jump there like we want.
|
||||
JX.DOM.scrollTo(JX.$(id.substr(1)));
|
||||
JX.DOM.scrollTo(target);
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in a new issue