XWIKI-24755: XClass properties can only be reordered with a mouse in the class editor

* Added a reorderControls widget adding a move up and a move down button to each item of a list
* Made both buttons move the item on click and on the up and down arrow keys
* Announced the new position of the moved item in a live region
* Announced that the item is already at the end of the list it is moving towards
* Replaced a pending announcement when a new move is performed
* Replaced the class editor move handle span with that widget
* Removed the default border and padding of the move buttons
* Left room for the focus indicator of the last property tool
* Documented that the sortable keeps its default cancel option to protect the move buttons
* Added the move up and move down translation keys
* Deprecated the moveProperty.handle.label translation key
* Added the property move actions and the move button getters to ClassEditPage
* Started the property drag from the property title in movePropertyBefore
* Added reorderPropertyWithoutDragging to EditClassIT

Author: Sereza5

xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-resources/src/main/resources/flamingo/javascript.vm

@@ -104,6 +104,7 @@
     'xwiki-job-runner': "#removeJsSuffix($services.webjars.url('org.xwiki.platform:xwiki-platform-job-webjar', ""jobRunner${jsExtension}""))",
     'xwiki-locale-picker': $xwiki.getSkinFile('localePicker.js', true),
     'xwiki-meta': $xwiki.getSkinFile('js/xwiki/meta.js'),
+    'xwiki-reorder-controls': $xwiki.getSkinFile('uicomponents/widgets/reorderControls.js'),
     'xwiki-selectize': $xwiki.getSkinFile('uicomponents/suggest/xwiki.selectize.js'),
     'xwiki-suggestAttachments-bundle': $xwiki.getSkinFile('uicomponents/suggest/suggestAttachments.js'),
     'xwiki-tree-finder': "#removeJsSuffix($services.webjars.url('org.xwiki.platform:xwiki-platform-tree-webjar', ""finder${jsExtension}""))",

xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/EditClassIT.java

@@ -212,6 +212,72 @@ void reorderProperty(TestUtils setup, TestReference reference) throws Exception
         assertEquals(List.of("testA", "testC", "testB"), classEditPage.getProperties());
     }
 
+    @Test
+    @Order(6)
+    void reorderPropertyWithoutDragging(TestUtils setup, TestReference reference) throws Exception
+    {
+        setup.rest().savePage(reference, "Some content", "");
+        ClassEditPage classEditPage = setup.editClass(reference);
+        classEditPage.addProperty("testA", "Number");
+        classEditPage.addProperty("testB", "Number");
+        classEditPage.addProperty("testC", "Number");
+
+        // A property added without reloading the editor gets its move buttons too, and they move it right away.
+        // The properties added before it keep exactly one pair of buttons, however many times the editor adds one.
+        assertEquals(List.of("testA", "testB", "testC"), classEditPage.getProperties());
+        assertEquals(2, classEditPage.getMovePropertyButtonCount("testA"));
+        assertEquals(2, classEditPage.getMovePropertyButtonCount("testC"));
+        classEditPage.movePropertyUp("testC");
+        assertEquals(List.of("testA", "testC", "testB"), classEditPage.getProperties());
+        classEditPage.movePropertyDown("testC");
+        assertEquals(List.of("testA", "testB", "testC"), classEditPage.getProperties());
+
+        classEditPage.clickSaveAndView();
+
+        classEditPage = setup.editClass(reference);
+        assertEquals(List.of("testA", "testB", "testC"), classEditPage.getProperties());
+
+        // Both move controls are buttons, and each one has an accessible name and a tooltip naming the property it
+        // moves.
+        assertEquals("button", classEditPage.getMovePropertyUpButton("testC").getTagName());
+        assertEquals("move up", classEditPage.getMovePropertyUpButtonName("testC"));
+        assertEquals("Move property testC up", classEditPage.getMovePropertyUpButtonTooltip("testC"));
+        assertEquals("move down", classEditPage.getMovePropertyDownButtonName("testA"));
+        assertEquals("Move property testA down", classEditPage.getMovePropertyDownButtonTooltip("testA"));
+
+        // A single click moves the property, and the button keeps the focus so that several moves can be performed in
+        // a row.
+        classEditPage.movePropertyUp("testC");
+        assertEquals(List.of("testA", "testC", "testB"), classEditPage.getProperties());
+        assertTrue(classEditPage.isMovePropertyUpButtonFocused("testC"));
+        classEditPage.waitForReorderAnnouncement("Property moved to position 2 out of 3");
+        classEditPage.movePropertyUp("testC");
+        assertEquals(List.of("testC", "testA", "testB"), classEditPage.getProperties());
+
+        // The first property cannot be moved further up, and says so rather than staying silent.
+        classEditPage.movePropertyUp("testC");
+        assertEquals(List.of("testC", "testA", "testB"), classEditPage.getProperties());
+        classEditPage.waitForReorderAnnouncement("Property already at the top of the list");
+
+        // The last property cannot be moved further down, and says so too.
+        classEditPage.movePropertyDown("testB");
+        assertEquals(List.of("testC", "testA", "testB"), classEditPage.getProperties());
+        classEditPage.waitForReorderAnnouncement("Property already at the bottom of the list");
+
+        // The arrow keys move the property too, and both buttons answer both keys.
+        classEditPage.movePropertyDownWithKeyboard("testC");
+        assertEquals(List.of("testA", "testC", "testB"), classEditPage.getProperties());
+        classEditPage.movePropertyUpWithKeyboard("testC");
+        assertEquals(List.of("testC", "testA", "testB"), classEditPage.getProperties());
+        classEditPage.movePropertyDownWithKeyboard("testC");
+        assertEquals(List.of("testA", "testC", "testB"), classEditPage.getProperties());
+
+        classEditPage.clickSaveAndView();
+
+        classEditPage = setup.editClass(reference);
+        assertEquals(List.of("testA", "testC", "testB"), classEditPage.getProperties());
+    }
+
     private DocumentReference getTestClassDocumentReference(TestReference reference)
     {
         return new DocumentReference("TestClass", reference.getLastSpaceReference());

xwiki-platform-core/xwiki-platform-oldcore/src/main/resources/ApplicationResources.properties

@@ -1346,7 +1346,13 @@ core.editors.class.addProperty.inProgress=Adding property...
 core.editors.class.addProperty.done=Property added
 core.editors.class.addProperty.failed=Failed:
 
-core.editors.class.moveProperty.handle.label=Drag and drop to change the order
+core.editors.class.moveProperty.up.text=move up
+core.editors.class.moveProperty.up.tooltip=Move property {0} up
+core.editors.class.moveProperty.up.boundary=Property already at the top of the list
+core.editors.class.moveProperty.down.text=move down
+core.editors.class.moveProperty.down.tooltip=Move property {0} down
+core.editors.class.moveProperty.down.boundary=Property already at the bottom of the list
+core.editors.class.moveProperty.moved=Property moved to position {0} out of {1}
 
 core.editors.class.deleteProperty.text=delete
 core.editors.class.deleteProperty.tooltip=Delete property {0}
@@ -5732,6 +5738,14 @@ admin.pagerights.info=These rights apply on this page only.
 admin.pagerights.infoNonTerminalDoc=They do not affect the {0}children{1}.
 core.edit.autosave.every=every
 
+#######################################
+## until 18.8.0RC1
+#######################################
+## The move handle this key labeled was replaced by a move up button and a move down button, each carrying its own
+## label and tooltip, so the replacements are core.editors.class.moveProperty.up.tooltip and the down equivalent.
+#@deprecated core.editors.class.moveProperty.up.tooltip
+core.editors.class.moveProperty.handle.label=Drag and drop to change the order
+
 ## Used to indicate where deprecated keys end
 #@deprecatedend
 

xwiki-platform-core/xwiki-platform-test/xwiki-platform-test-ui/src/main/java/org/xwiki/test/ui/po/editor/ClassEditPage.java

@@ -22,6 +22,7 @@
 import java.util.List;
 
 import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.Actions;
 import org.openqa.selenium.support.FindBy;
@@ -139,11 +140,193 @@ public List<String> getProperties()
      */
     public void movePropertyBefore(String propertyToMove, String propertyBefore)
     {
-        WebElement moveToolSource = getDriver()
-            .findElementWithoutWaiting(By.id("xproperty_" + propertyToMove))
-            .findElement(By.cssSelector(".tool.move"));
         WebElement target = getDriver().findElementWithoutWaiting(By.id("xproperty_" + propertyBefore));
-        new Actions(getDriver().getWrappedDriver()).dragAndDrop(moveToolSource, target).perform();
+        new Actions(getDriver().getWrappedDriver()).dragAndDrop(getPropertyDragSource(propertyToMove), target)
+            .perform();
+    }
+
+    /**
+     * Clicks the move up button of the given property. The property keeps its position when it is already the first
+     * one.
+     *
+     * @param propertyToMove the property to move one position up
+     * @since 18.8.0RC1
+     */
+    public void movePropertyUp(String propertyToMove)
+    {
+        getMovePropertyUpButton(propertyToMove).click();
+    }
+
+    /**
+     * Clicks the move down button of the given property. The property keeps its position when it is already the last
+     * one.
+     *
+     * @param propertyToMove the property to move one position down
+     * @since 18.8.0RC1
+     */
+    public void movePropertyDown(String propertyToMove)
+    {
+        getMovePropertyDownButton(propertyToMove).click();
+    }
+
+    /**
+     * Presses the up arrow key on the move buttons of the given property, which both answer it. The property keeps its
+     * position when it is already the first one.
+     *
+     * @param propertyToMove the property to move one position up
+     * @since 18.8.0RC1
+     */
+    public void movePropertyUpWithKeyboard(String propertyToMove)
+    {
+        getMovePropertyUpButton(propertyToMove).sendKeys(Keys.ARROW_UP);
+    }
+
+    /**
+     * Presses the down arrow key on the move buttons of the given property, which both answer it. The property keeps
+     * its position when it is already the last one.
+     *
+     * @param propertyToMove the property to move one position down
+     * @since 18.8.0RC1
+     */
+    public void movePropertyDownWithKeyboard(String propertyToMove)
+    {
+        getMovePropertyDownButton(propertyToMove).sendKeys(Keys.ARROW_DOWN);
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the button moving the given property one position up
+     * @since 18.8.0RC1
+     */
+    public WebElement getMovePropertyUpButton(String propertyName)
+    {
+        return getPropertyContainer(propertyName).findElement(By.cssSelector(".reorder-control-up"));
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the button moving the given property one position down
+     * @since 18.8.0RC1
+     */
+    public WebElement getMovePropertyDownButton(String propertyName)
+    {
+        return getPropertyContainer(propertyName).findElement(By.cssSelector(".reorder-control-down"));
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return whether the button moving the given property up currently holds the focus
+     * @since 18.8.0RC1
+     */
+    public boolean isMovePropertyUpButtonFocused(String propertyName)
+    {
+        return getMovePropertyUpButton(propertyName).equals(getDriver().switchTo().activeElement());
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the accessible name of the button moving the given property up, which is displayed to screen readers
+     *     only
+     * @since 18.8.0RC1
+     */
+    public String getMovePropertyUpButtonName(String propertyName)
+    {
+        return getButtonName(getMovePropertyUpButton(propertyName));
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the accessible name of the button moving the given property down, which is displayed to screen readers
+     *     only
+     * @since 18.8.0RC1
+     */
+    public String getMovePropertyDownButtonName(String propertyName)
+    {
+        return getButtonName(getMovePropertyDownButton(propertyName));
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the tooltip of the button moving the given property up
+     * @since 18.8.0RC1
+     */
+    public String getMovePropertyUpButtonTooltip(String propertyName)
+    {
+        return getMovePropertyUpButton(propertyName).getDomAttribute("title");
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the tooltip of the button moving the given property down
+     * @since 18.8.0RC1
+     */
+    public String getMovePropertyDownButtonTooltip(String propertyName)
+    {
+        return getMovePropertyDownButton(propertyName).getDomAttribute("title");
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the number of move buttons displayed for the given property, which stays at two however many times
+     *     properties are added to the editor
+     * @since 18.8.0RC1
+     */
+    public int getMovePropertyButtonCount(String propertyName)
+    {
+        return getPropertyContainer(propertyName).findElements(By.cssSelector(".reorder-control")).size();
+    }
+
+    /**
+     * @param button one of the move buttons
+     * @return the accessible name of the given button
+     */
+    private String getButtonName(WebElement button)
+    {
+        return button.findElement(By.className("sr-only")).getDomProperty("textContent");
+    }
+
+    /**
+     * Waits until the move buttons have announced the given text to the screen readers.
+     *
+     * @param announcement the expected announcement
+     * @since 18.8.0RC1
+     */
+    public void waitForReorderAnnouncement(String announcement)
+    {
+        getDriver().waitUntilCondition(driver -> announcement.equals(getReorderAnnouncement()));
+    }
+
+    /**
+     * Reads the announcement without waiting for it. The live region is emptied as soon as a move is performed and
+     * filled again shortly after, so this returns an empty string when it is called right after a move. Prefer
+     * {@link #waitForReorderAnnouncement(String)}, which waits for the announcement to be set.
+     *
+     * @return the text currently held by the live region the move buttons announce their moves in
+     * @since 18.8.0RC1
+     */
+    public String getReorderAnnouncement()
+    {
+        return getDriver().findElementWithoutWaiting(By.id("reorder-controls-live-region"))
+            .getDomProperty("textContent");
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the part of the property title a drag has to start from, the move buttons being operated by a click
+     *     rather than by a drag
+     */
+    private WebElement getPropertyDragSource(String propertyName)
+    {
+        return getPropertyContainer(propertyName).findElement(By.cssSelector(".xproperty-title h2"));
+    }
+
+    /**
+     * @param propertyName the name of a property of this class
+     * @return the element holding the title and the content of the given property
+     */
+    private WebElement getPropertyContainer(String propertyName)
+    {
+        return getDriver().findElementWithoutWaiting(By.id("xproperty_" + propertyName));
     }
 
     private FormContainerElement getForm()

xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war/src/main/webapp/resources/js/xwiki/editors/dataeditors.css

@@ -223,8 +223,10 @@ The default state is collapsed for xobjects and xproperties, but expanded for xc
   font-weight: 900;
 }
 
+/* The right padding leaves room for the focus indicator of the last tool, which the hidden overflow would cut off. */
 .xproperty-title .tools {
   align-self: center;
+  padding-right: 3px;
 }
 
 .xproperty-title .tools .tool {
@@ -235,10 +237,16 @@ The default state is collapsed for xobjects and xproperties, but expanded for xc
 }
 
 .xproperty-title .tools .move {
-  cursor: move;
   color: $theme.linkColor;
 }
 
+/* The move tools are buttons, so their default border and padding have to be removed to align them with the other
+tools. The background is already reset by the rule above. */
+.xproperty-title .tools button.tool {
+  border: none;
+  padding: 0;
+}
+
 .xproperty-title .tools .delete {
   color: $theme.notificationErrorColor;
 }