Bugzilla – Full Text Bug Listing
| Summary: | Selection in navigation tree does not invoke an action (must click on the link text rather than the just the row) | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Steve Northover <steve_northover> |
| Component: | Client | Assignee: | libing wang <libingw> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P2 | CC: | curtis.windatt.public, libingw, mamacdon, Silenio_Quarti |
| Version: | unspecified | ||
| Target Milestone: | 9.0 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
|
Description
Steve Northover
diff --git a/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js b/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js
index 7381319..d8f126f 100644
--- a/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js
+++ b/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js
@@ -192,6 +192,17 @@
});
this._clickListener = function(evt) {
+ if (!(evt.metaKey || evt.altKey || evt.shiftKey || evt.ctrlKey)) {
+ var navHandler = _self.getNavHandler();
+ if (navHandler && navHandler.getSelectionPolicy() !== "cursorOnly") {
+ var link = lib.$("a", evt.target);
+ if (link) {
+ // Run the link action when user clicks anywhere in the item cell
+// link.click();
+ window.location.href = link.href;
+ }
+ }
+ }
if (evt.target.tagName === "A") { //$NON-NLS-0$
var temp = evt.target;
while (temp) {
This means that row selection only works when you hold down a modifier key. So effectively there is no way to highlight an item (for deletion, renaming, etc) with a touch device -- you will always open it instead. So perhaps lists that support row selection should provide a generic "selection mode" toggle that would allow actions to be taken on the row without following links. I know some mobile apps provide this. (This means we're almost back at the very early Orion UI, which had a checkbox on every list item!) Is seems to me that you will still be able to select, but the behavior will be that something opens when before it did not. I am likely missing something. Can you give me some exact steps that will fail on my touch device? I can try it out with the patch. (In reply to Steve Northover from comment #3) > Is seems to me that you will still be able to select, but the behavior will > be that something opens when before it did not. I am likely missing > something. > > Can you give me some exact steps that will fail on my touch device? I can > try it out with the patch. I see your point. You can still take the action, but it requires opening the file first as a side effect. Silenio, this one is really bad for me. No other editor that I am using has this behavior including some of the ones I have tried on the desktop. Each time I hit it, I think that Orion is broken and has not fetched the contents of my file. Talked to Silenio today and we think we should fix it now. More and more people complained about this. As the selection model is generic to all pages, I will tried to test all the corner cases to see if other selection stories are broken after the changes I will make. Working on it now... I've changed the patch a little bit and tested on my self hosting.
Here is the new patch.
diff --git a/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js b/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js
index a414da3..2695e61 100644
--- a/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js
+++ b/bundles/org.eclipse.orion.client.ui/web/orion/explorers/explorer-table.js
@@ -192,18 +192,18 @@
});
this._clickListener = function(evt) {
- if (evt.target.tagName === "A") { //$NON-NLS-0$
- var temp = evt.target;
- while (temp) {
- if (temp._item) {
- break;
- }
- temp = temp.parentNode;
- }
- if (temp && temp._item) {
- _self.onLinkClick({type: "linkClick", item: temp._item}); //$NON-NLS-0$
- }
- }
+ if (!(evt.metaKey || evt.altKey || evt.shiftKey || evt.ctrlKey)) {
+ var navHandler = _self.getNavHandler();
+ if (navHandler && navHandler.getSelectionPolicy() !== "cursorOnly") {
+ var link = lib.$("a", evt.target);
+ if (link) {
+ window.location.href = link.href;
+ _self._clickLink(link);
+ return;
+ }
+ }
+ }
+ _self._clickLink(evt.target);
};
var parent = lib.node(this.parentId);
if (parent) {
@@ -226,6 +226,20 @@
}
mExplorer.Explorer.prototype.destroy.call(this);
},
+ _clickLink: function(linkElement) {
+ if (linkElement.tagName === "A") { //$NON-NLS-0$
+ var temp = linkElement;
+ while (temp) {
+ if (temp._item) {
+ break;
+ }
+ temp = temp.parentNode;
+ }
+ if (temp && temp._item) {
+ this.onLinkClick({type: "linkClick", item: temp._item}); //$NON-NLS-0$
+ }
+ }
+ },
onLinkClick: function(clickEvent) {
this.dispatchEvent(clickEvent);
},
I've tested on both editor page and git page. We will lose multiple selection on tablets because there is no SHIFT and CTRL keys in tablets. Before the change, user can 1. Tap on the empty space to select a item 2. Tap on another item to do multiple selection 3. Tap on a selected item to deselect. Also found another thing for keyboard users when they just use up and down arrow keys to move the selection. This is not related to the change I've made because it only changed the click behavior: If you move the selection by arrow keys, it does not open that file unless you press enter key. If a user depends on left hand selection to see which file is being opened, this case is a bit confusing. But again, this is an existing issue. For now, I would detect that we are on a tablet (or don't have a keyboard) and run the old code. I'm not sure what the error you describe in the keyboard flow is but it seems as if it exists regardless of this change. Fixed with http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=bc02d92bbd3e927831d870753c932c790bb9ff16. We are losing the multiple selection on IPAD now but we can address it in another bug if we need to. We need some agressive tests on beta3 on Monday for this fix though I've tested a lot on Orion. This might have been an excellent thing to dark launch behind a flag so that it could be turned on or off easily. More discussion of this next week. |