Part of #2006152: [meta] Don't call theme() directly anywhere outside drupal_render()

CommentFileSizeAuthor
#16 replace_theme_drupal_render_views_ui-2006974-16.patch7.99 KBjlbellido
#8 2006974-8.patch9.46 KBthedavidmeister
#8 2006974-interdiff-1-8.txt918 bytesthedavidmeister
#1 2006974-1.patch9.2 KBthedavidmeister

Comments

thedavidmeister’s picture

thedavidmeister commented
Status: Active » Needs review
StatusFileSize
new 2006974-1.patch9.2 KB

patch

Status: Needs review » Needs work

The last submitted patch, 2006974-1.patch, failed testing.

thedavidmeister’s picture

thedavidmeister commented
Status: Needs work » Needs review

#1: 2006974-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2006974-1.patch, failed testing.

c4rl’s picture

c4rl commented
Title: Remove direct theme() calls from views_ui » Replace theme() with drupal_render() in views_ui.module

Retitiling

thedavidmeister’s picture

thedavidmeister commented
Status: Needs work » Needs review

#1: 2006974-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2006974-1.patch, failed testing.

thedavidmeister’s picture

thedavidmeister commented
Status: Needs work » Needs review
StatusFileSize
new 2006974-interdiff-1-8.txt918 bytes
new 2006974-8.patch9.46 KB
thedavidmeister’s picture

thedavidmeister commented
Issue tags: +Novice, +Needs manual testing
kay_v’s picture

kay_v commented
Assigned: Unassigned » kay_v
kay_v’s picture

kay_v commented
Assigned: kay_v » Unassigned
jenlampton’s picture

jenlampton
she / her
commented
Issue tags: -Needs manual testing

Not sure why this one is marked needs manual testing, there's no twig files in here to test. Just removed calls to theme? What needs manual testing?

adamcowboy’s picture

adamcowboy commented
Status: Needs review » Reviewed & tested by the community

I applied the patch and added a new view. Everything looks fine.

thedavidmeister’s picture

thedavidmeister commented

@jenlampton - I was told in IRC that "everything Views related had to be manually tested as different issues had broken the UI a few times before"

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll...

git ac https://drupal.org/files/2006974-8.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  9687  100  9687    0     0   3791      0  0:00:02  0:00:02 --:--:--  7979
error: patch failed: core/modules/views_ui/lib/Drupal/views_ui/Form/BreakLockForm.php:84
error: core/modules/views_ui/lib/Drupal/views_ui/Form/BreakLockForm.php: patch does not apply
error: patch failed: core/modules/views_ui/lib/Drupal/views_ui/ViewListController.php:38
error: core/modules/views_ui/lib/Drupal/views_ui/ViewListController.php: patch does not apply
jlbellido’s picture

jlbellido
Primary language Spanish
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new replace_theme_drupal_render_views_ui-2006974-16.patch7.99 KB

Re rolled, i have tested manually and it looks fine. Removed tag and running test for this re-roll.

adamcowboy’s picture

adamcowboy commented
Status: Needs review » Reviewed & tested by the community

Applied the patch and tested. Everything looks fine. The previous guy manually tested it and he did good work.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed e36b512 and pushed to 8.x. Thanks!

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