diff --git a/ui/TODO.otl b/ui/TODO.otl index f6405e6e..ac48cf2d 100755 --- a/ui/TODO.otl +++ b/ui/TODO.otl @@ -111,13 +111,34 @@ Roadmap [_] 32% Gen 3 current todo [_] 64% High priority [_] ASAP: Need visual indicators for long operations... - [_] BUG: (phantom) in single image mode shifting first image up to new ribbon errs... - | error location/reason: - | getImageGid(..) got something odd in the image gid attribute... + [X] BUG: shifting around images in very short ribbons produces lots of errors + | Procedure: + | - crop a ribbon of two-three images + | - start shifting | - | Race condition?? + | Problems: + | - wrong alignment + | - sometimes by shifting an image up the focus shifts to the + | upper ribbon | - | Can't reproduce this... need to test on a faster machine... + | The direct cause of the problem: + | - errors: + | - SyntaxError - likely the JSON parser in getImageGID() + | - TypeError - property of null + | + | Likely causes: + | - we access images BEFORE they are initialized properly... + | + | Solution strategy: + | - make aligning independent of DOM state where possible... + | - make parts of the code wait for updates to finish... + | + | + | Actual case: + | - this was not a problem with race conditions... + | - essentially all this took is a careful look at event and + | action ordering, and a forgotten filter... + | (filtering for images is a possible bug class) [_] 66% Bookmarks [X] toggle [X] save/load diff --git a/ui/data.js b/ui/data.js index f8fbad0a..2994d211 100755 --- a/ui/data.js +++ b/ui/data.js @@ -2380,7 +2380,6 @@ function setupData(viewer){ from = getRibbonIndex(from) //var ribbon = to to = getRibbonIndex(to) - var gid = getImageGID(image) var after = getGIDBefore(gid, to) diff --git a/ui/files.js b/ui/files.js index 39d2b2ae..bef666db 100755 --- a/ui/files.js +++ b/ui/files.js @@ -305,8 +305,11 @@ var saveFileData = makeFileSaver( }) +// NOTE: this will set the updated flag ONLY of out of cropped mode... function dataUpdated(){ - fileUpdated('Data') + if(!isViewCropped()){ + fileUpdated('Data') + } } diff --git a/ui/ribbons.js b/ui/ribbons.js index 7e6bc9c9..6b3155ff 100755 --- a/ui/ribbons.js +++ b/ui/ribbons.js @@ -128,7 +128,7 @@ function getImageGID(image){ } -// Get marks associated with image... +// Get mark elements associated with image... // // img can be: // - literal gid @@ -256,6 +256,9 @@ function getImageBefore(image, ribbon){ } +// NOTE: this just shifts the image, it does not care about either +// aligning nor focus... +// NOTE: the shiftedImage event is fired BEFORE any ribbons are removed... function shiftTo(image, ribbon){ var target = getImageBefore(image, ribbon) var cur_ribbon = getRibbon(image) @@ -265,6 +268,9 @@ function shiftTo(image, ribbon){ if(target.length == 0){ image.prependTo($(ribbon)) + // insert the image... + // NOTE: we need to take care to insert the image not just after the + // target, but also after the target's marks... } else { var target_marks = getImageMarks(target).last() image.insertAfter( @@ -277,6 +283,7 @@ function shiftTo(image, ribbon){ // move the marks... image.after(marks) + // NOTE: this is intentionally fired BEFORE removing a ribbon... $('.viewer').trigger('shiftedImage', [image, cur_ribbon, ribbon]) // if removing last image out of a ribbon, remove the ribbon.... @@ -1176,13 +1183,10 @@ function shiftImageTo(image, direction, moving, force_create_ribbon){ var b = moving == 'prev' ? 'nextAll' : 'prevAll' var target = image[a]('.image').first() - target = target.length == 0 ? image[b]().first() : target - - // XXX should this be in here or coupled later via an event??? - //flashIndicator(direction) + target = target.length == 0 ? image[b]('.image').first() : target shiftImage(direction, image, force_create_ribbon) - // XXX does this need to be animated??? + return centerView(focusImage(target), 'css') } function shiftImageUp(image, moving){