I think it would be slightly more intuitive if tabledrag does not offer re-ordering of table rows when there is only one item in the list.

CommentFileSizeAuthor
#11 core-js-hide-tabledrag-1016056-11.patch753 bytesnod_
PASSED: [[SimpleTest]]: [MySQL] 36,666 pass(es). View
#10 core-js-hide-tabledrag-1016056-10.patch850 bytesnod_
PASSED: [[SimpleTest]]: [MySQL] 33,360 pass(es). View
#2 core-js-hide-tabledrag-1016056-2.patch1.07 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 33,343 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

sun’s picture

Version: 7.0-rc4 » 8.x-dev
Issue tags: -user experience +Needs usability review, +Usability

Sounds like a good idea to me.

nod_’s picture

Status: Active » Needs review
FileSize
1.07 KB
PASSED: [[SimpleTest]]: [MySQL] 33,343 pass(es). View
Bojhan’s picture

Works here, I think this is a good idea too. Do you actually remove it, or just hide it through JS, because our screenreader users shouldn't have it either?

Bojhan’s picture

nod_’s picture

I filter the selector, If there is only one row with the draggable class, the behavior is not run on it. So yeah, It'll remove all the stuff for screenreaders too.

Bojhan’s picture

I imagine you can keep it from being rendered at all? If you can fix that part its RTBC from my perspective, sure someone can give it a code review then.

nod_’s picture

I don't understand which behavior you're describing, have you tried the patch ?

sun’s picture

I think this patch, and thus avoiding tabledrag via JS, is the right approach. AJAX operations might be adding or changing the page content, so when re-attaching behaviors on the same page, we want to actually see the tabledrag.

That said, don't we need to hide the weight column in the table?

Bojhan’s picture

Ahh, I think I was just confused by the "weight" table row still being there. Is there a reason that is still there?

nod_’s picture

FileSize
850 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,360 pass(es). View

You're right, I wanted to avoid all processing but it just end up being confusing. Here is a better one.

nod_’s picture

FileSize
753 bytes
PASSED: [[SimpleTest]]: [MySQL] 36,666 pass(es). View

reroll

Bojhan’s picture

Status: Needs review » Reviewed & tested by the community

:)

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks!

David_Rothstein’s picture

Title: Hide tabledrag handle and "Show row weights" when there is only one item in list » (Regression: drag and drop broken) Hide tabledrag handle and "Show row weights" when there is only one item in list
Category: feature » bug
Priority: Normal » Major
Status: Fixed » Needs work
Issue tags: +regression

This seems to have broken lots of stuff - for example, on a default Drupal install go to admin/structure/types/manage/page/display and you can't drag the single field to "Hidden" anymore, or click "Edit shortcuts", delete one of the two shortcuts, and try to drag the remaining one to disabled, etc.

Basically, if there's more than one region to drag to, the drag handles should never be removed. If that's not easy to fix, this patch should be rolled back.

Also, for what it's worth, the "Show row weights" links are still showing for me in these cases.

***

I was actually wondering if it made sense to backport the original patch here to Drupal 7 (since it's a borderline bugfix, I think)... but obviously no chance until the above issues are resolved :)

nod_’s picture

So I was going to write a patch and started my reply but the main issue here is that the way we disable/enable rows is clunky. There is no point in fixing this patch, it's the whole UI that needs fixing as it's pretty much useless on mobile too.

I'm meeting up with Bojhan at drupaljam this friday to see what can be done about tabledrag UX. We'll add that to the list of things to talk about.

Sorry about the UI breakage, please rollback this patch. We'll take it in a followup of #1524414: Rewrite tabledrag.js to use jQuery UI.

David_Rothstein’s picture

Status: Needs work » Reviewed & tested by the community

OK, thanks. I guess #11 is RTBC for the rollback; don't think we need a new patch since the patch can just be reverse-applied.

catch’s picture

Title: (Regression: drag and drop broken) Hide tabledrag handle and "Show row weights" when there is only one item in list » Hide tabledrag handle and "Show row weights" when there is only one item in list
Priority: Major » Normal
Status: Reviewed & tested by the community » Active

Rolled this back.

catch’s picture

Issue tags: +Needs manual testing
nod_’s picture

Status: Active » Needs work
andypost’s picture

Suppose new behavior should enable table drag in case table gets updated via ajax

andypost’s picture

droplet’s picture

Hide tabledrag handle: AGREED

Hide "Show row weights":

For example. we have 2 diff tables, and fetch the data from these 2 tables and sort by "weight". Remove the row weight column, I have no way to set the value.

tstoeckler’s picture

@droplet: I don't think that use-case is common enough that it should introduce a column that is useless in 95% of the cases. In case it's hard to alter the weight column back in in your specific case, then it should be a follow-up issue to make that easy. But we should still remove it IMO.

@nod_/all: What is the status of this issue? I just stumbled on this coming from #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController) and still think it is very useful.

nod_’s picture

Issue summary: View changes
Status: Needs work » Closed (duplicate)
Related issues: +#1016056: Hide tabledrag handle and "Show row weights" when there is only one item in list

Not touching tabledrag anymore.

David_Rothstein’s picture

Status: Closed (duplicate) » Needs work
Issue tags: +needs backport to D7

That issue is for 8.1.x (or possibly later) and doesn't have a patch. Plus I think we do still want to backport this to Drupal 7, if it's possible to fix it without breaking other things?

nod_’s picture

I'm not confident it's possible without some serious code change. I gave it an honest go at #15 and it got out of hands the fact I still remember it should give a clue :þ

droplet’s picture

If I'm honest, I'd say close it as "won't fix". Keeping UI consistency is another point for usability. (Even good for theme styles)

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • catch committed 9f31a02 on 8.3.x
    Rollback #1016056 due to a regression.
    
    
  • Dries committed cde3375 on 8.3.x
    - Patch #1016056 by nod_: added Hide tabledrag handle and 'Show row...

  • catch committed 9f31a02 on 8.3.x
    Rollback #1016056 due to a regression.
    
    
  • Dries committed cde3375 on 8.3.x
    - Patch #1016056 by nod_: added Hide tabledrag handle and 'Show row...

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • catch committed 9f31a02 on 8.4.x
    Rollback #1016056 due to a regression.
    
    
  • Dries committed cde3375 on 8.4.x
    - Patch #1016056 by nod_: added Hide tabledrag handle and 'Show row...

  • catch committed 9f31a02 on 8.4.x
    Rollback #1016056 due to a regression.
    
    
  • Dries committed cde3375 on 8.4.x
    - Patch #1016056 by nod_: added Hide tabledrag handle and 'Show row...

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.