Problem/Motivation

Follow-up to #2036195: Remove views-more.html.twig and replace with #type link render arrays

+++ b/core/modules/views/views.module
@@ -279,6 +275,15 @@ function views_theme_suggestions_comment_alter(array &$suggestions, array $varia
+ if (!empty($variables['element']['#type']) && $variables['element']['#type'] == 'more_link' && !empty($variables['element']['#view']) && $variables['element']['#view'] instanceof \Drupal\views\ViewExecutable) {

There should not be full namespace, add "use" statement and it's strange that there's no interface for ViewExecutable

From @andypost

Coding standards reference: https://www.drupal.org/node/1353118

Proposed resolution

Fix it

Remaining tasks

None

User interface changes

n/a

API changes

n/a

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task, just a coding standards fix
Issue priority Normal, nothing's broken
Disruption No disruption at all, just a coding standards fix.

Comments

yogen.prasad’s picture

Assigned: Unassigned » yogen.prasad
yogen.prasad’s picture

yogen.prasad’s picture

Assigned: yogen.prasad » Unassigned
Status: Active » Needs review
andypost’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/views.module
@@ -266,7 +266,7 @@ function views_preprocess_comment(&$variables) {
-  if (!empty($comment->view) && $comment->view->storage->id()) {
+ if (!empty($variables['element']['#type']) && $variables['element']['#type'] == 'more_link' && !empty($variables['element']['#view']) && $variables['element']['#view'] instanceof \Drupal\views\ViewExecutable) {

indent should be 2 spaces, and needs add use statement to not use full namespace for Viewexecutble

yogen.prasad’s picture

Assigned: Unassigned » yogen.prasad
willzyx’s picture

we are talking about fix code in views_theme_suggestions_container_alter(), isn't it?
in the patch we are modifying views_theme_suggestions_comment_alter()

yogen.prasad’s picture

I think we need to modify code in both views_theme_suggestions_container_alter() and views_theme_suggestions_comment_alter(),

yogen.prasad’s picture

Assigned: yogen.prasad » Unassigned
Status: Needs work » Needs review
star-szr’s picture

Title: Add use statement for \Drupal\views\ViewExecutable in the views.module » Don't use full namespace for \Drupal\views\ViewExecutable in views.module
Status: Needs review » Needs work
Issue tags: +Quick fix

Use statement is already there, so retitling.

Thanks for working on this @yogen.prasad.

+++ b/core/modules/views/views.module
@@ -266,7 +266,7 @@ function views_preprocess_comment(&$variables) {
-  if (!empty($comment->view) && $comment->view->storage->id()) {
+  if (!empty($variables['element']['#type']) && $variables['element']['#type'] == 'more_link' && !empty($variables['element']['#view']) && $variables['element']['#view'] instanceof ViewExecutable){

This change needs to be reverted, it's not correct, we only need to change the one line in views_theme_suggestions_container_alter().

andypost’s picture

Looks we have no test coverage s/element/elements - You checking wrong property

+++ b/core/modules/views/views.module
@@ -266,7 +266,7 @@ function views_preprocess_comment(&$variables) {
-  if (!empty($comment->view) && $comment->view->storage->id()) {
+  if (!empty($variables['element']['#type']) && $variables['element']['#type'] == 'more_link' && !empty($variables['element']['#view']) && $variables['element']['#view'] instanceof ViewExecutable){

not sure the change makes sense, it was supposed to check that $comment has a view as property!

Also data in "elements" not in "element"?

deepakaryan1988’s picture

Fixed the issue as mentioned in comment#9

star-szr’s picture

Status: Needs work » Reviewed & tested by the community

Yup, #11 does the trick I'd say :)

deepakaryan1988’s picture

That's great @Cottser !!
:)

star-szr’s picture

Issue summary: View changes

Adding a basic beta evaluation and updating the issue summary.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 39e97f3 on 8.0.x
    Issue #2481751 by yogen.prasad, deepakaryan1988, Cottser, andypost,...

Status: Fixed » Closed (fixed)

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