Download & Extend
Issues

Refactor views_ui_rearrange_form to remove theme function for table render

Posted by damiankloip on February 27, 2013 at 2:21pm
Project:Drupal core
Version:8.x-dev
Component:views_ui.module
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)
Issue tags:Quick fix, VDC

Issue Summary

We currently have a theme_views_ui_rearrange_form function that renders the form build to be a table. Nowadays we can just do this natively using the form/render api in our form callback.

There is a related issue, #1921774: Javascript on reorder modal form tables is missing., meaning the javascript for tabledrag is not included when first opening the modal. This issue can still carry on without that though.

I have also created #1929070: Refactor views_ui_rearrange_filter_form to remove theme function for table rendering for the filter rearrange form.

AttachmentSizeStatusTest resultOperations
vdc.remove-theme-views-ui-rearrange-form.patch4.97 KBIdlePASSED: [[SimpleTest]]: [MySQL] 52,152 pass(es).View details
Login or register to post comments

Comments

#1

Posted by tim.plunkett on February 27, 2013 at 2:43pm

#1919142: Convert Views UI AJAX forms to use FormInterface moves all of this around, and is pretty much done at this point. Just needs an RTBC... :)

But yes, let's PLEASE do this! Much better.

#2

Posted by damiankloip on March 4, 2013 at 6:36pm
Status:needs review» reviewed & tested by the community

Ok, that patch got in now, so I've rerolled these changes to be in Drupal\views_ui\Form\Ajax\Rearrange::buildForm().

AttachmentSizeStatusTest resultOperations
1929064-2.patch4.74 KBIdlePASSED: [[SimpleTest]]: [MySQL] 52,252 pass(es).View details

#3

Posted by damiankloip on March 4, 2013 at 7:57pm
Status:reviewed & tested by the community» needs review

#4

Posted by dawehner on March 4, 2013 at 8:17pm
Status:needs review» reviewed & tested by the community

Oh i figured out why the tabledrag doesn't work: The problem is the insertion order of the commands.
As the settings comes after the actual replaced HTML the Drupal.behavior.attach functions are executed before the settings are in there. Interesting, I will create a follow up for that.

This good is looking really great, just removing the unused code above it.

AttachmentSizeStatusTest resultOperations
drupal-1929064-4.patch5.16 KBIdlePASSED: [[SimpleTest]]: [MySQL] 54,177 pass(es).View details
interdiff.txt982 bytesIgnored: Check issue status.NoneNone

#5

Posted by dawehner on March 4, 2013 at 8:26pm

#1848880: Test order of AJAX commands: add Ajax\FrameworkTest::testOrder(), strengthen testLazyLoad(), fix JS settings not being prepended has a fix for that.

#6

Posted by damiankloip on March 5, 2013 at 9:06am

Even less code, nice!

#7

Posted by dawehner on March 18, 2013 at 8:43am

#4: drupal-1929064-4.patch queued for re-testing.

#8

Posted by xjm on March 24, 2013 at 7:14pm

#4: drupal-1929064-4.patch queued for re-testing.

#9

Posted by xjm on March 27, 2013 at 2:00am
Issue tags:+Quick fix

#10

Posted by xjm on April 5, 2013 at 8:20pm

#4: drupal-1929064-4.patch queued for re-testing.

#11

Posted by alexpott on April 10, 2013 at 10:53pm
Status:reviewed & tested by the community» fixed

Committed dee9ef6 and pushed to 8.x. Thanks!

#12

Posted by andypost on April 13, 2013 at 5:11pm
Status:fixed» needs review

Quick follow-up to clean-up hook_theme()

AttachmentSizeStatusTest resultOperations
1929064-12.patch626 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 54,341 pass(es).View details

#13

Posted by dawehner on April 13, 2013 at 6:54pm
Status:needs review» reviewed & tested by the community

Great! Thanks for fixing this.

#14

Posted by alexpott on April 14, 2013 at 9:02pm
Status:reviewed & tested by the community» fixed

Committed dc8aeca and pushed to 8.x. Nice catch - thanks!

#15

Posted by System Message on April 28, 2013 at 9:10pm
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.