FLUID-3391: The wrapping of selection and reordering items via keyboard should be configurable

Description

The wrapping of selection and reordering items via keyboard should be configurable

Some users, for example those using AT's, may not want reorderer to wrap around. Currently if you move an item down it will wrap to the top after reaching the end. This may make it difficult for some users to know what the boundaries are.

Attachments

• 20100105_wrappingSwitch.patch
• FLUID-3391.patch
• FLUID-3391.v10.patch
• FLUID-3391.v11.patch
• FLUID-3391.v12.patch
• FLUID-3391.v2.patch
• FLUID-3391.v3.patch
• FLUID-3391.v4.patch
• FLUID-3391.v5.patch
• FLUID-3391.v6.patch
• FLUID-3391.v7.patch
• FLUID-3391.v8.patch
• FLUID-3391.v9.patch

Comments

• Colin Clark commented 2010-03-30T19:24:47.000-0400

  This will be a critical fix for future versions of Decapod, as well as being a great a11y improvement. I've assigned it to Jonathan to help coordinate it with Decapod.

• Justin Obara commented 2010-05-07T14:00:53.712-0400

  a11y issue

• Justin Obara commented 2010-09-17T11:13:13.035-0400

  Link to conversations from the fluid-work mailing list
  http://old.nabble.com/Wrapping-around-in-ListReorderer-to26634522.html#a26750933

• Justin Obara commented 2010-10-04T15:24:30.330-0400

  "Bug Parade Infusion 1.3"

• Golam Chowdhury commented 2010-10-21T17:07:22.347-0400

  FLUID-3391: FLUID-3391.patch gives the user the ability to turn on and off elements wrapping via keyboard.

• Golam Chowdhury commented 2010-10-22T11:30:24.429-0400

  FLUID-3391: FLUID-3391.v2.patch made some code changes. Also it includes everything from FLUID-3391.patch.

• Golam Chowdhury commented 2010-10-22T12:14:07.347-0400

  FLUID-3391.v3.patch made some code changes. Includes everything from previous patches.

• Antranig Basman commented 2010-10-22T13:50:39.067-0400

  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?

• Golam Chowdhury commented 2010-10-27T11:31:22.904-0400

  FLUID-3391: FLUID-3391.v4.patch contains all the changes from v1-v3 patches and necessary test cases. Note: still in process of refactoring some of the test cases

• Golam Chowdhury commented 2010-10-28T18:09:01.857-0400

  FLUID-3391.v5.patch which contains everything from previous versions including test cases which was re-factored.

• Golam Chowdhury commented 2010-10-29T15:22:22.418-0400

  FLUID-3391.v6.patch which contains everything from previous versions including test cases which was re-factored.

• Golam Chowdhury commented 2010-11-01T15:07:32.200-0400

  FLUID-3391.v7.patch which contains everything from previous versions and re-factor of codes.

• Golam Chowdhury commented 2010-11-01T16:36:13.584-0400

  FLUID-3391.v8.patch contains all changes from previous versions.

• Golam Chowdhury commented 2010-11-01T17:37:47.472-0400

  FLUID-3391.v9.patch contains everything from previous versions.

• Golam Chowdhury commented 2010-11-02T16:08:45.002-0400

  FLUID-3391.v10.patch contains everything from previous versions and re-factored codes.

• Antranig Basman commented 2010-11-04T04:42:31.254-0400

  Hi golam - my suggestion for patch "10" is that it should be possible to make a better consolidation of the three "step" test functions than "commonAction". These functions overall all take the same arguments, and the points of variability are
  i) the different markup selected for "focusItem" which could be supplied as a function handle, ii) the different argument to bindReorderer, and iii) the difference in the precise method of simulating the keypress - which I'm not actually sure why this varies - why does moduleLayout use compositeKey and the other two use arg.key (ctrlKeyEvent) - would it be possible to consolidate these also?

  A suitable signature might be
  fluid.testUtils.reorderer.stepReorderer(container, options) where options contains
  {
  reordererFn
  reordererOptions
  direction
  expectOrderFn
  expectedOrderArrays
  focusElementFn

  you can then use fluid.invokeGlobalFunction(options.reordererFn, [container, options.reordererOptions]) to construct the reorderer etc.
  then options.expectOrderFn(that, expectedOrderArrays[etc.]

• Golam Chowdhury commented 2010-11-04T18:07:51.722-0400

  FLUID-3391.v11.patch contains everything from previous patches and some re-factored codes.

• Antranig Basman commented 2010-11-05T05:11:31.783-0400

  Hi there Golam - v11 of the patch looks good. Just to tidy up i) since just one function call now exists to "commonAction" it should simply be folded into the main body of the caller, especially since it has no particularly well-defined function of its own, ii) please ensure that all tabs are converted to spaces in the patch (see if someone can help you out with Eclipse/Aptana settings) and then we can get the patch committed - thanks for all your hard work on this, Antranig.

• Golam Chowdhury commented 2010-11-05T11:33:00.188-0400

  FLUID-3391.v12.patch contains everything from previous patches and some code cleanup with JSLint.

• Antranig Basman commented 2010-11-08T04:28:51.030-0500

  Committed at revision 10190