Fix Page Designer ImageWithText Link component

[adamraya]

Description

The initial simple solution is to keep using Chakra UI Link component in the PD ImageWithText Link component.

A more elaborated solution uses isAbsoluteURL util to conditionally use Chakra UI Link component for absolute URLs and our custom react-router Link component for relative URLs.

TODO:

• Remove the example page viewer page and its route in routes.jsx.

Types of Changes

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Documentation update
• Breaking change (could cause existing functionality to not work as expected)
• Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

• Removing a public function or component or prop
• Adding a required argument to a function
• Changing the data type of a function parameter or return value
• Adding a new peer dependency to package.json

Changes

• Updated ImageWithText Link component.

How to Test-Drive This PR

• Start template-retail-react-app and load http://localhost:3000/page-viewer/get-ready-for-summer.
• Click on the images links, and verify all links use hard navigation except the Kids link which does a soft navigation to a relative URL.

Checklists

General

• Changes are covered by test cases
• CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

• There are no changes to UI

or...

• Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
• Changes comply with WCAG 2.0 guidelines levels A and AA
• Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

• Changes include a UI text update in the Retail React App (which requires translation)

[bendvc]

If you look at the Link component from the template components folder you'll see that that component has similar logic built into it that you are duplicating here maybe? 🤔

I think there is a prop called useNavLink that you might be able to take advantage of.

[adamraya]

I don't think that is the case.

Reading our custom link component uses a react-router Link or NavLink component (used for the Breadcrumbs menu) with the Chakra Link styles.

pwa-kit/packages/template-retail-react-app/app/components/link/index.jsx

Lines 18 to 19 in 3396472

	<ChakraLink
	as={useNavLink ? NavSPALink : SPALink}

There's no logic to assert if the url string is absolute, and if so, use a Chakra UI Link component instead of react-router component.

[adamraya]

The prop useNavLink only determines if we use react-router's Link vs NavLink component.

[breadadams]

I've resolved a similar issue recently by using the as prop. Since the ...props spread is after the as={useNavLink ? ...} prop (L21 in the snippet below), you can override it by passing a custom as to the Link component.

pwa-kit/packages/template-retail-react-app/app/components/link/index.jsx

Lines 13 to 26 in 6b9d436

	const Link = React.forwardRef(({href, to, useNavLink = false, ...props}, ref) => {
	const _href = to || href
	const {buildUrl} = useMultiSite()
	const updatedHref = buildUrl(_href)
	return (
	<ChakraLink
	as={useNavLink ? NavSPALink : SPALink}
	{...(useNavLink && {exact: true})}
	{...props}
	to={updatedHref}
	ref={ref}
	/>
	)
	})

Meaning you could possibly adjust your current logic to something like:

import Link from '../path/to/Link'

const linkProps = ({ITCLink}) => {
    return isAbsoluteURL(ITCLink) ? { as: ChakraLink } : {}
}

() => <Link {...linkProps(/* args */ )} >...</Link>

Additionally, I added a LinkComponent?: ComponentWithAs<'a'> prop to our <Link/>, which has a default value of <ChakraLink />. This allows us to override the UI of the link (e.g. using Chakra UI's <LinkOverlay /> component) while retaining the react-router link logic from the as prop.

[adamraya]

Thanks @breadadams for the feedback. I played with the idea and you're right, we can override our custom react-router <Link /> component logic with the ChakraLink logic by using as={ChakraLink}. The declared as prop passed to the <Link /> component will override the as prop with ...props spread.
But then we need to modify the <Link /> to conditionally use href or to props depending on if we use ChakraLink or a react-router-like Link component.

The LinkComponent idea to override styles and keep the react-router Link logic makes sense as well, but today we use the layerStyle prop pattern to override styles.

The scope of the PR is to fix the Link in the ImageWithText Page designer component. We plan to extract the Page designer code from the template in the future. In this context, I think it's better to solve the problem in the ImageWithText component file. But both ideas are good suggestions to make our custom <Link /> component more flexible in future extensibility work.