1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Fix two bugs with DraggableList

Summary:
Ref T1344. This fixes two issues with DraggableList:

  - In lists which allowed it, you could drag the top item above itself and get a dashed-border ghost item. This didn't make sense and didn't behave well. Just don't treat this operation as valid.
  - In lists which allowed it, you could drag any non-top item to the topmost position, then drag it to an invalid position. The dashed-border ghost item would not be removed properly if this happend.
  - Also fix some minor leftovers with Celerity.

Test Plan:
  - Dragged the first item above itself; now an invalid operation with no ghost.
  - Dragged another item to the first position then back to its original position; ghost vanishes.
  - Clean lint.

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1344

Differential Revision: https://secure.phabricator.com/D7939
This commit is contained in:
epriestley 2014-01-13 12:23:20 -08:00
parent 996930da2a
commit 7ffddcd136
3 changed files with 14 additions and 5 deletions

View file

@ -213,7 +213,7 @@ final class CelerityResourceMap {
* is unknown.
*/
public function getRequiredSymbolsForName($name) {
$hash = idx($this->symbolMap, $name);
$hash = idx($this->nameMap, $name);
if ($hash === null) {
return null;
}

View file

@ -147,7 +147,7 @@ final class PhabricatorJavelinLinter extends ArcanistLinter {
$path);
$need = $external_classes;
$resource_name = substr($path, strlen('webroot'));
$resource_name = substr($path, strlen('webroot/'));
$requires = $celerity->getRequiredSymbolsForName($resource_name);
if (!$requires) {
$requires = array();
@ -170,7 +170,7 @@ final class PhabricatorJavelinLinter extends ArcanistLinter {
// If JS requires CSS, just assume everything is fine.
unset($requires[$key]);
} else {
$symbol_path = 'webroot'.$requires_name;
$symbol_path = 'webroot/'.$requires_name;
list($ignored, $req_install) = $this->getUsedAndInstalledSymbolsForPath(
$symbol_path);
if (array_intersect_key($req_install, $external_classes)) {

View file

@ -197,12 +197,21 @@ JX.install('DraggableList', {
break;
}
// If the dragged row is the first row, don't allow it to be dragged
// into the first position, since this operation doesn't make sense.
if (cur_target === null) {
var first_item = targets[targets.length - 1].item;
if (dragging === first_item) {
cur_target = false;
}
}
// If we've selected a new target, update the UI to show where we're
// going to drop the row.
if (cur_target != target) {
if (cur_target !== target) {
if (target) {
if (target !== false) {
JX.DOM.remove(ghost);
}