ADAPT-1178 Asset download in new window #88

yvonnetangsu · 2020-10-30T06:40:52Z

READY FOR REVIEW

Summary

Add asset download option for CTA link component
If link is an asset link, make it open in new tab

Review By (Date)

When does this need to be reviewed by?

Criticality

How critical is this PR on a 1-10 scale? Also see Severity Assessment.
E.g., it affects one site, or every site and product?

Review Tasks

Setup tasks and/or behavior to test

Go to https://deploy-preview-88--adapt-giving.netlify.app/how-to-make-a-gift/planned-giving/bequests-and-estate-plan-gifts/
scroll down to the PDF colored cards in "more information" section
Click on one card and see that the pdf opens in a new tab
Go to https://deploy-preview-88--adapt-giving.netlify.app/areas-to-support/
Click on the 3rd card (Stanford Libraries), check that the links does NOT open in new tab.
Go to https://deploy-preview-88--adapt-giving.netlify.app/test-items/yvonne/yvonne-test-landing-page/
Scroll to the poster "This is a poster headline...."
Click on the call to action button and see that the pdf opens in a new window.

Front End Validation

Is the markup using the appropriate semantic tags and passes HTML validation?
Cross-browser testing has been performed?
Automated accessibility scans performed?
Manual accessibility tests performed?
Design is approved by @ user?

Backend / Functional Validation

Code

Are the naming conventions following our standards?
Does the code have sufficient inline comments?
Is there anything in this code that would be hidden or hard to discover through the UI?
Are there any code smells?
Are tests provided? eg (unit, behat, or codeception)

Code security

Are all forms properly sanitized?
Any obvious security flaws or new areas for attack?

General

Is there anything included in this PR that is not related to the problem it is trying to solve?
Is the approach to the problem appropriate?

Affected Projects or Products

Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

JIRA ticket(s)
Other PRs
Any other contextual information that might be helpful (e.g., description of a bug that this PR fixes, new functionality that it adds, etc.)
Anyone who should be notified? (@mention them here)

Resources

AMP Tool
Accessibility Manual Test Script
HTML Validator
Browserstack and link to Browserstack Credentials

sherakama · 2020-10-30T19:11:03Z

src/components/cards/oodSupportCard.js

@@ -37,7 +37,9 @@ const OodSupportCard = (props) => {
        }
        {(props.blok.link.linktype === "url" || props.blok.link.linktype === "asset") &&
          <a href={props.blok.link.url ? props.blok.link.url : props.blok.link.cached_url}
-             className={`ood-support-card__link su-bg-${props.blok.backgroundColor}`}>
+             className={`ood-support-card__link su-bg-${props.blok.backgroundColor}`}
+             {...props.blok.link.linktype === "asset" ? {target : "_blank"} : {}}


Where did this request come from? Normally we favor allowing the user to choose their way of getting the content.

Came out of the client meeting yesterday, and @kerri-augenstein agrees that this could be a special case (for asset link only).

hmm. seems NNgroup agrees. https://www.nngroup.com/articles/new-browser-windows-and-tabs/#:~:text=In%20general%2C%20avoid%20PDF%20for,in%20new%20tabs%20on%20desktop.

That should be fine with this site then, since they said their target audience aren't mobile users.

sherakama · 2020-10-30T19:12:41Z

src/components/simple/ctaLink.js

-        {props.blok.link.linktype === "url" &&
-          <a href={props.blok.link.url}
+        {(props.blok.link.linktype === "url" || props.blok.link.linktype === "asset") &&
+          <a href={props.blok.link.url ? props.blok.link.url : props.blok.link.cached_url}


What is cached_url?

That's how Storyblok references the link if it's an asset link

Here's the link object if it's an asset link

Ah found the doxn: https://www.storyblok.com/faq/link-object-history

Thanks.

sherakama

GTG

yvonnetangsu added 2 commits October 29, 2020 23:32

If support card is linked to an asset, have it open in new window

d0dcdef

Add asset download feature to CTA component

aec7b8d

yvonnetangsu requested review from katrialesser and sherakama October 30, 2020 06:47

sherakama reviewed Oct 30, 2020

View reviewed changes

yvonnetangsu requested a review from sherakama October 30, 2020 19:53

sherakama approved these changes Oct 30, 2020

View reviewed changes

yvonnetangsu merged commit feefadf into main Oct 30, 2020

yvonnetangsu deleted the ADAPT-1178-asset-download branch October 30, 2020 20:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADAPT-1178 Asset download in new window #88

ADAPT-1178 Asset download in new window #88

yvonnetangsu commented Oct 30, 2020 •

edited

Loading

sherakama Oct 30, 2020

yvonnetangsu Oct 30, 2020

sherakama Oct 30, 2020

yvonnetangsu Oct 30, 2020

sherakama Oct 30, 2020

yvonnetangsu Oct 30, 2020

yvonnetangsu Oct 30, 2020

sherakama Oct 30, 2020

sherakama left a comment

ADAPT-1178 Asset download in new window #88

ADAPT-1178 Asset download in new window #88

Conversation

yvonnetangsu commented Oct 30, 2020 • edited Loading

READY FOR REVIEW

Summary

Review By (Date)

Criticality

Review Tasks

Setup tasks and/or behavior to test

Front End Validation

Backend / Functional Validation

Code

Code security

General

Affected Projects or Products

Associated Issues and/or People

Resources

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sherakama left a comment

Choose a reason for hiding this comment

yvonnetangsu commented Oct 30, 2020 •

edited

Loading