Follow-up to #2349775: Remove classes from Views templates

I'm adding this issue to just copy the Views templates from the module directory to Classy. There are lots of classes scattered about Views, so it is unclear how long it may take to clean them out of core. With the aim of getting Classy shippable, lets copy the templates as a first step. This is similar to how we treated System.

This issue copies all views templates other than the field templates because they are currently output as theme functions by default.

The original issue, 2349775, we can still use to do the removal of classes.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task
Issue priority Not critical because nothing is broken.
Unfrozen changes Unfrozen because it only changes markup/theme.
Prioritized changes The main goal of this issue is to improve the themer experience..
Disruption This should not be disruptive.
Files: 
CommentFileSizeAuthor
#16 copy_views_templates_to-2424533-16.patch20.74 KBdavidhernandez
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,905 pass(es). View
#13 copy_views_templates_to-2424533-13.patch4.37 KBdavidhernandez
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,862 pass(es). View
#9 views.diff23.69 KBmortendk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,964 pass(es), 39 fail(s), and 0 exception(s). View
copyviews-toclassy.patch5.21 KBdavidhernandez
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,805 pass(es), 161 fail(s), and 4 exception(s). View

Comments

davidhernandez’s picture

mortendk’s picture

Status: Needs review » Reviewed & tested by the community

yes lets use same approach as we did with system - copy over all the templates, so we can begin the work on the css & classes
while we clean up core's templates at the same time ... if the test is green offcourse

Status: Reviewed & tested by the community » Needs work

The last submitted patch, copyviews-toclassy.patch, failed testing.

mortendk’s picture

which its not huh?

davidhernandez’s picture

This is weird. On things like UserSwitchTest, the markup is coming out escaped.

davidhernandez’s picture

This seems to be related to the Views theme function conversions which have not all been completed yet.

Status: Needs work » Needs review

mortendk queued copyviews-toclassy.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, copyviews-toclassy.patch, failed testing.

mortendk’s picture

Status: Needs work » Needs review
FileSize
23.69 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,964 pass(es), 39 fail(s), and 0 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 9: views.diff, failed testing.

davidhernandez’s picture

Interesting; fewer fails than last time, but I think this still isn't going to work until #2348747: Convert theme_views_view_fields() to 100% Twig: remove the theme function gets in.

mortendk’s picture

he he yeah probably - but im still gonna see if we can get em all fixed ;)
no guts no glory!

davidhernandez’s picture

Status: Needs work » Needs review
FileSize
4.37 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,862 pass(es). View

Checking to see if this works without the -field and -fields template.

If we can get green patch while leaving out the field templates, we should proceed without them. Core currently uses theme functions for fields and not the actual templates, for performance reasons. There are issues to work on it, but it may not be possible to make those conversions before release. In light of that, proceeding without the field templates doesn't hurt much. They do not contain CSS classes, so we do not need Stark vs. Classy version of them anyway.

mortendk’s picture

+1 on this im to tired right now to review anything though ;)

Cottser’s picture

Status: Needs review » Needs work

+1 to this patch/approach for the stated reasons. However, the "Default theme implementation" lines need to be replaced, see #2452363: Classy's @file docblocks shouldn't say "Default theme implementation…".

You can just replace "Default theme implementation" with "Theme override" throughout.

davidhernandez’s picture

Status: Needs work » Needs review
FileSize
20.74 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,905 pass(es). View

I change the opening description to say "Theme override". I also removed the @ingroup which shouldn't be there either.

Cottser’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Yes, thanks @davidhernandez I forgot about that pesky thing!

Updating the issue summary to explain why we are skipping the field templates.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This is only moving front end templates. Committed f2ed4ce and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed f2ed4ce on 8.0.x
    Issue #2424533 by davidhernandez, mortendk: Copy views templates to...

Status: Fixed » Closed (fixed)

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