Updated: Comment #N

Problem/Motivation

views "more link" has a wrapper div with .more-link, that class should be on the link instead of the wrapper & reducing the divitis

Proposed resolution

Add more-link to the link & remove the wrapper div

<div class="more-link">
  <a href="{{ more_url }}">
    {{ link_text }}
  </a>
</div>
<a href="{{ more_url }}" class="more-link">
  {{ link_text }}
</a>

Remaining tasks

User interface changes

API changes

Comments

Status: Needs review » Needs work

The last submitted patch, views-more-link.diff, failed testing.

mandar.harkare’s picture

Status: Needs work » Needs review
StatusFileSize
new481 bytes

I am really not sure why patch by mortendk failed.

Status: Needs review » Needs work

The last submitted patch, 2: drupal-2226923-more-link.patch, failed testing.

mandar.harkare’s picture

Status: Needs work » Needs review
StatusFileSize
new445 bytes

New line was missing.

Status: Needs review » Needs work

The last submitted patch, 4: drupal-2226923-more-link-4.patch, failed testing.

pakmanlh’s picture

Status: Needs work » Needs review
StatusFileSize
new2.75 KB

Here I attach the rerolled patch modifying the test which failed, changing the div removed selector by only the anchor tag.

star-szr’s picture

Status: Needs review » Needs work
+++ b/core/modules/views/templates/views-more.html.twig
@@ -11,8 +11,6 @@
-<div class="more-link">
-  <a href="{{ more_url }}">
...
   </a>
-</div>

What about this:

<a href="{{ more_url }}" class="more-link">{{ link_text }}</a>

All on one line and not indented.

galooph’s picture

Assigned: Unassigned » galooph
galooph’s picture

The changes to the tests in the patch from #6 look sound to me. I've run the Drupal\views\Tests\Plugin\DisplayTest->testReadMore() test locally and it passes ok.

galooph’s picture

Assigned: galooph » Unassigned
Status: Needs work » Needs review
StatusFileSize
new2.76 KB

Here's the patch from #6 but with the link all on one line and not indented, as per #7.

Status: Needs review » Needs work

The last submitted patch, 10: drupal-2226923-more-link-10.patch, failed testing.

pakmanlh’s picture

Status: Needs work » Needs review
StatusFileSize
new2.76 KB

Here the patch again.

star-szr’s picture

Status: Needs review » Reviewed & tested by the community

Looks great to me.

tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs screenshots

In HEAD this is display: block, because its a div. With this, its now displayed inline, which is a regression.
I'd say won't fix this, but if not, then put display:block in system.theme.css

mortendk’s picture

Status: Needs work » Needs review
StatusFileSize
new3.2 KB

display: block added to .more-link

do we need screenshots as well ?

star-szr’s picture

Screenshots yes please. And we have .more-link for non-Views more links so those need to be checked as well.

Thanks @tim.plunkett for catching this!

mortendk’s picture

and heres the screenshots

mortendk’s picture

Heres the screenshots:

BArtik

before

after

Block before

Block after

seven

before

after

Stark

before

after

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs screenshots

Looks great, thanks for the fix and screenshots!

manuel garcia’s picture

Issue tags: +Views dreammarkup
manuel garcia’s picture

Issue tags: -Views dreammarkup +VDC
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Awesome, thanks. :)

Committed and pushed to 8.x.

  • Commit 242ac39 on 8.x by webchick:
    Issue #2226923 by pakmanlh, mandar.harkare, mortendk, galooph: Views:...

Status: Fixed » Closed (fixed)

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