Hide
added a comment -
Hi Golam - this patch is pretty good and addresses the more difficult part of the problem, which is modifying the geometric strategies to account for wrap locking. However, in our original implementation it was revealed that a lot of subtle bugs can result from trying to use a Geometric strategy in the "covariant" direction of the Reorderer, that is, the direction specified as the dominant orientation in the top-level configuration. So what needs to be done is to create a "disableWrap" version of the logical strategy to be used in this case - so, for example, on this section of the patch for Reorderer.js
@@ -675,7 +684,7 @@
that.getRelativePosition =
fluid.reorderer.relativeInfoGetter(options.orientation,
- fluid.reorderer.LOGICAL_STRATEGY, fluid.reorderer.SHUFFLE_GEOMETRIC_STRATEGY,
+ dropManager.disableWrap ? fluid.reorderer.SHUFFLE_GEOMETRIC_STRATEGY : fluid.reorderer.LOGICAL_STRATEGY , fluid.reorderer.SHUFFLE_GEOMETRIC_STRATEGY,
dropManager, dom);
you subsitute a goemetric strategy for the logical strategy in the case of disableWrap which will cause problems.
So, what the implementation requires is a modification of the getRelativeElement function (currently on line 455 of GeometricManager.js) to have signature
function getRelativeElement(element, direction, elements, disableWrap) {
Although I said on our phone chat that we might want to create new versions of all the strategies, I am thinking now this is probably rather silly. What there should be instead is a modification of all of the signatures of the existing strategies - so that we now have
that.logicalFrom = function (element, direction, includeLocked, disableWrap)
that.lockedWrapFrom = function (element, direction, includeLocked, disableWrap)
etc.
since actually EVERY strategy we have currently can be varied in this way.
I couldn't see from your patch how it set about disabling wrapping for the ListReorderer - the "disableWrap" argument is present as configuration, and handed down to the dropManager - but as it stands now, the logicalFrom strategy would still perform wrapping? Which configurations of the Reorderer did you test your patch with?
Also, I don't believe we want this new behaviour to become the default - we can't change the behaviour of all reorderers for all existing users.
Also I want to query the subtask I see on this issue "Turning wrapping on/off via a function call after initialization" - what is motivating this task?
Show
added a comment - Hi Golam - this patch is pretty good and addresses the more difficult part of the problem, which is modifying the geometric strategies to account for wrap locking. However, in our original implementation it was revealed that a lot of subtle bugs can result from trying to use a Geometric strategy in the "covariant" direction of the Reorderer, that is, the direction specified as the dominant orientation in the top-level configuration. So what needs to be done is to create a "disableWrap" version of the logical strategy to be used in this case - so, for example, on this section of the patch for Reorderer.js
@@ -675,7 +684,7 @@
that.getRelativePosition =
fluid.reorderer.relativeInfoGetter(options.orientation,
- fluid.reorderer.LOGICAL_STRATEGY, fluid.reorderer.SHUFFLE_GEOMETRIC_STRATEGY,
+ dropManager.disableWrap ? fluid.reorderer.SHUFFLE_GEOMETRIC_STRATEGY : fluid.reorderer.LOGICAL_STRATEGY , fluid.reorderer.SHUFFLE_GEOMETRIC_STRATEGY,
dropManager, dom);
you subsitute a goemetric strategy for the logical strategy in the case of disableWrap which will cause problems.
So, what the implementation requires is a modification of the getRelativeElement function (currently on line 455 of GeometricManager.js) to have signature
function getRelativeElement(element, direction, elements, disableWrap) {
Although I said on our phone chat that we might want to create new versions of all the strategies, I am thinking now this is probably rather silly. What there should be instead is a modification of all of the signatures of the existing strategies - so that we now have
that.logicalFrom = function (element, direction, includeLocked, disableWrap)
that.lockedWrapFrom = function (element, direction, includeLocked, disableWrap)
etc.
since actually EVERY strategy we have currently can be varied in this way.
I couldn't see from your patch how it set about disabling wrapping for the ListReorderer - the "disableWrap" argument is present as configuration, and handed down to the dropManager - but as it stands now, the logicalFrom strategy would still perform wrapping? Which configurations of the Reorderer did you test your patch with?
Also, I don't believe we want this new behaviour to become the default - we can't change the behaviour of all reorderers for all existing users.
Also I want to query the subtask I see on this issue "Turning wrapping on/off via a function call after initialization" - what is motivating this task?
Bug
Closed
Major
Open
Next steps after acceptance are to expose the functionality via a function to toggle it on/off
and integrate it into the example.
After this the wrapping switch should be introduced for other reorderers