Skip to content

Commit

Permalink
feat: Use SPA navigation from datasets list to Explore (#20890)
Browse files Browse the repository at this point in the history
* feat: Use SPA navigation from datasets list to Explore

* fix lint

* Fix lint

* Fix type

* Make external links work without protocols

* add comment
  • Loading branch information
kgabryje authored Aug 1, 2022
1 parent 90e2d82 commit 6ec164e
Show file tree
Hide file tree
Showing 5 changed files with 203 additions and 2 deletions.
59 changes: 59 additions & 0 deletions superset-frontend/src/components/GenericLink/GenericLink.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import { GenericLink } from './GenericLink';

test('renders', () => {
render(<GenericLink to="/explore">Link to Explore</GenericLink>, {
useRouter: true,
});
expect(screen.getByText('Link to Explore')).toBeVisible();
});

test('navigates to internal URL', () => {
render(<GenericLink to="/explore">Link to Explore</GenericLink>, {
useRouter: true,
});
const internalLink = screen.getByTestId('internal-link');
expect(internalLink).toHaveAttribute('href', '/explore');
});

test('navigates to external URL', () => {
render(
<GenericLink to="https://superset.apache.org/">
Link to external website
</GenericLink>,
{ useRouter: true },
);
const externalLink = screen.getByTestId('external-link');
expect(externalLink).toHaveAttribute('href', 'https://superset.apache.org/');
});

test('navigates to external URL without host', () => {
render(
<GenericLink to="superset.apache.org/">
Link to external website
</GenericLink>,
{ useRouter: true },
);
const externalLink = screen.getByTestId('external-link');
expect(externalLink).toHaveAttribute('href', '//superset.apache.org/');
});
52 changes: 52 additions & 0 deletions superset-frontend/src/components/GenericLink/GenericLink.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { Link, LinkProps } from 'react-router-dom';
import { isUrlExternal, parseUrl } from 'src/utils/urlUtils';

export const GenericLink = <S,>({
to,
component,
replace,
innerRef,
children,
...rest
}: React.PropsWithoutRef<LinkProps<S>> &
React.RefAttributes<HTMLAnchorElement>) => {
if (typeof to === 'string' && isUrlExternal(to)) {
return (
<a data-test="external-link" href={parseUrl(to)} {...rest}>
{children}
</a>
);
}
return (
<Link
data-test="internal-link"
to={to}
component={component}
replace={replace}
innerRef={innerRef}
{...rest}
>
{children}
</Link>
);
};
54 changes: 54 additions & 0 deletions superset-frontend/src/utils/urlUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* 'License'); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { isUrlExternal, parseUrl } from './urlUtils';

test('isUrlExternal', () => {
expect(isUrlExternal('http://google.com')).toBeTruthy();
expect(isUrlExternal('https://google.com')).toBeTruthy();
expect(isUrlExternal('//google.com')).toBeTruthy();
expect(isUrlExternal('google.com')).toBeTruthy();
expect(isUrlExternal('www.google.com')).toBeTruthy();
expect(isUrlExternal('mailto:mail@example.com')).toBeTruthy();

// treat all urls starting with protocol or hostname as external
// such urls are not handled well by react-router Link component
expect(isUrlExternal('http://localhost:8888/port')).toBeTruthy();
expect(isUrlExternal('https://localhost/secure')).toBeTruthy();
expect(isUrlExternal('http://localhost/about')).toBeTruthy();
expect(isUrlExternal('HTTP://localhost/about')).toBeTruthy();
expect(isUrlExternal('//localhost/about')).toBeTruthy();
expect(isUrlExternal('localhost/about')).toBeTruthy();

expect(isUrlExternal('/about')).toBeFalsy();
expect(isUrlExternal('#anchor')).toBeFalsy();
});

test('parseUrl', () => {
expect(parseUrl('http://google.com')).toEqual('http://google.com');
expect(parseUrl('//google.com')).toEqual('//google.com');
expect(parseUrl('mailto:mail@example.com')).toEqual(
'mailto:mail@example.com',
);
expect(parseUrl('google.com')).toEqual('//google.com');
expect(parseUrl('www.google.com')).toEqual('//www.google.com');

expect(parseUrl('/about')).toEqual('/about');
expect(parseUrl('#anchor')).toEqual('#anchor');
});
33 changes: 32 additions & 1 deletion superset-frontend/src/utils/urlUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
import { JsonObject, QueryFormData, SupersetClient } from '@superset-ui/core';
import {
isDefined,
JsonObject,
QueryFormData,
SupersetClient,
} from '@superset-ui/core';
import rison from 'rison';
import { isEmpty } from 'lodash';
import {
Expand Down Expand Up @@ -175,3 +180,29 @@ export function getDashboardPermalink({
anchor,
});
}

const externalUrlRegex =
/^([^:/?#]+:)?(?:(\/\/)?([^/?#]*))?([^?#]+)?(\?[^#]*)?(#.*)?/;

// group 1 matches protocol
// group 2 matches '//'
// group 3 matches hostname
export function isUrlExternal(url: string) {
const match = url.match(externalUrlRegex) || [];
return (
(typeof match[1] === 'string' && match[1].length > 0) ||
match[2] === '//' ||
(typeof match[3] === 'string' && match[3].length > 0)
);
}

export function parseUrl(url: string) {
const match = url.match(externalUrlRegex) || [];
// if url is external but start with protocol or '//',
// it can't be used correctly with <a> element
// in such case, add '//' prefix
if (isUrlExternal(url) && !isDefined(match[1]) && !url.startsWith('//')) {
return `//${url}`;
}
return url;
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import ImportModelsModal from 'src/components/ImportModal/index';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import { isUserAdmin } from 'src/dashboard/util/permissionUtils';
import { GenericLink } from 'src/components/GenericLink/GenericLink';
import AddDatasetModal from './AddDatasetModal';

import {
Expand Down Expand Up @@ -289,7 +290,11 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
},
},
}: any) => {
const titleLink = <a href={exploreURL}>{datasetTitle}</a>;
const titleLink = (
// exploreUrl can be a link to Explore or an external link
// in the first case use SPA routing, else use HTML anchor
<GenericLink to={exploreURL}>{datasetTitle}</GenericLink>
);
try {
const parsedExtra = JSON.parse(extra);
return (
Expand Down

0 comments on commit 6ec164e

Please sign in to comment.