Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #3020522: Read more link is shown twice in featured #1161

Merged
merged 3 commits into from Dec 27, 2018
Merged

Issue #3020522: Read more link is shown twice in featured #1161

merged 3 commits into from Dec 27, 2018

Conversation

frankgraave
Copy link
Contributor

@frankgraave frankgraave commented Dec 14, 2018 •
edited by jaapjan

Problem

The "Read More" link is shown twice in Featured blocks on the Social Landing Page. This is happening because certain template files in the social_landing_page module are extended from node--featured.html.twig where the this one already has the card__actionbar with the card__link. These elements were also in the extended template file, which caused it to show twice.

Solution

Remove the card__link element from node--topic--featured.html.twig and node-landing-page--featured.html.twig since only these two had the duplicate elements.

Issue tracker

https://www.drupal.org/project/social/issues/3020522

How to test

  • Check out the changes
  • Create a landing page with featured items, test any or all kinds.
  • Verify that you only have one Read More link in the featured card (on all content types).

Release notes

Fixed an issue where featured blocks in landing pages showed the Read More link twice.

@frankgraave
Issue 3020522 by frankgraave: remove double elements that showed the …
a7624da 
…read more link twice in featured blocks

Signed-off-by: frankgraave <frankgraave@gmail.com>
@frankgraave
Issue 3020522 by frankgraave: also remove the card__link from node pa…
7e9520c 
…ge featured

Signed-off-by: frankgraave <frankgraave@gmail.com>
@frankgraave frankgraave added the type: bug Fixes a bug in Open Social label Dec 14, 2018
@Kingdutch
Copy link
Member

This seems to have been introduced in #998

@robertragas
Copy link
Contributor

Looks logical, but not sure why this got introduced in the first place.

@Kingdutch
Copy link
Member

Kingdutch commented Dec 18, 2018 •
edited

As per Issue #2991405:

The .card__link element is supposed to be used in a .card__actionbar element that is next to the .teaser__content element.

This affects the node--featured.html.twig, group--featured.html.twig and profile--featured.html.twig files of the social_landing_page module.

This is because we're using Bootstrap under the hood and Bootstrap expects its cards to be laid out this way to ensure that all the paddings are correct.

It looks like in that original fix the links in the places of this PR were missed so removing that here is the way to go : )

We can actually get away with removing these blocks from node--event--featured.html.twig and profile--featured.html.twig. There the blocks were properly changed but they don't actually differ from what they're inheriting.

@Kingdutch
Issue #3020522 by frankgraave, robertragas, kingdutch: remove double …
534c649 
…read me links from landingpages.

This commit removes the duplicate block link code from the landing page
featured templates.
Kingdutch
Kingdutch approved these changes Dec 18, 2018
View reviewed changes
Copy link
Member

@Kingdutch Kingdutch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes I mentioned and tested the changes by Frank.

Looks good to me!

@jaapjan
Copy link
Contributor

jaapjan commented Dec 24, 2018

@Kingdutch so this is reviewed and can be merged?

@jaapjan jaapjan added this to the 4.2 milestone Dec 24, 2018
@jaapjan jaapjan merged commit 9e9f35b into 8.x-4.x Dec 27, 2018
@jaapjan jaapjan deleted the bugfix/3020522-read-more-twice branch December 27, 2018 14:50
@jaapjan
Copy link
Contributor

jaapjan commented Dec 27, 2018 •
edited

Merged it to 8.3, 8.4 and 8.5 branches

@jaapjan
Copy link
Contributor

jaapjan commented Dec 28, 2018

We can actually get away with removing these blocks from node--event--featured.html.twig and profile--featured.html.twig. There the blocks were properly changed but they don't actually differ from what they're inheriting.

Btw, this is not completely true. We need that code in profile--featured because otherwise you get that issue again:
https://www.drupal.org/project/social/issues/3019760

Will solve it in a separate PR.

@jaapjan jaapjan mentioned this pull request Dec 28, 2018
Merged
2 tasks
jaapjan pushed a commit that referenced this pull request Dec 28, 2018
@frankgraave @jaapjan
Issue #3020522 by frankgraave, jaapjan, robertragas, kingdutch: Read …
277d421 
…more link is shown twice in featured (#1161)

* Issue 3020522 by frankgraave: remove double elements that showed the read more link twice in featured blocks

Signed-off-by: frankgraave <frankgraave@gmail.com>

* Issue 3020522 by frankgraave: also remove the card__link from node page featured

Signed-off-by: frankgraave <frankgraave@gmail.com>

* Issue #3020522 by frankgraave, robertragas, kingdutch: remove double read me links from landingpages.

This commit removes the duplicate block link code from the landing page
featured templates.
@jaapjan jaapjan added the backport: verified This pull request has been back ported to an older minor version label Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kingdutch Kingdutch Kingdutch approved these changes

@ronaldtebrake ronaldtebrake Awaiting requested review from ronaldtebrake

Assignees
No one assigned
Labels
backport: verified This pull request has been back ported to an older minor version type: bug Fixes a bug in Open Social
Milestone
4.2
4 participants
@frankgraave @Kingdutch @robertragas @jaapjan