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.

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

Reader Manage Following: present a follow button if user appears to have entered a URL #13715

Merged
merged 9 commits into from
May 8, 2017
39 changes: 38 additions & 1 deletion client/lib/url/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/
import { parse as parseUrl } from 'url';
import { startsWith } from 'lodash';
import url from 'url';

/**
* Internal dependencies
Expand Down Expand Up @@ -107,6 +108,41 @@ function urlToSlug( url ) {
return withoutHttp( url ).replace( /\//g, '::' );
}

/**
* Checks if the supplied string appears to be a URL.
* Looks only for the absolute basics:
* - does it have a .suffix?
* - does it have at least two parts separated by a dot?
*
* @param {String} query The string to check
* @return {Boolean} Does it appear to be a URL?
*/
function resemblesUrl( query ) {
let parsedUrl = url.parse( query );

// Make sure the query has a protocol - hostname ends up blank otherwise
if ( ! parsedUrl.protocol ) {
parsedUrl = url.parse( 'http://' + query );
Copy link
Contributor

Choose a reason for hiding this comment

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

we may also want to verify that it's a protocol we support or can switch to. For example, gopher:// would be invalid, but we could switch feed:// to http://

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a cool idea. I won't do it in this particular spot (resemblesUrl) because it might be reused by other things, but worth us keeping in mind.

}

if ( ! parsedUrl.hostname || parsedUrl.hostname.indexOf( '.' ) === -1 ) {
return false;
}

// Check for a valid-looking TLD
if ( parsedUrl.hostname.lastIndexOf( '.' ) > ( parsedUrl.hostname.length - 3 ) ) {
return false;
}

// Make sure the hostname has at least two parts separated by a dot
const hostnameParts = parsedUrl.hostname.split( '.' ).filter( Boolean );
if ( hostnameParts.length < 2 ) {
return false;
}

return true;
}

export default {
isOutsideCalypso,
isExternal,
Expand All @@ -116,5 +152,6 @@ export default {
setUrlScheme,
urlToSlug,
// [TODO]: Move lib/route/add-query-args contents here
addQueryArgs
addQueryArgs,
resemblesUrl,
};
43 changes: 43 additions & 0 deletions client/lib/url/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
addSchemeIfMissing,
setUrlScheme,
urlToSlug,
resemblesUrl,
} from '../';

describe( 'withoutHttp', () => {
Expand Down Expand Up @@ -285,3 +286,45 @@ describe( 'urlToSlug()', () => {
expect( urlWithoutHttp ).to.equal( 'example.com::example::test123' );
} );
} );

describe( 'resemblesUrl()', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

beautiful.
this entire commit is beautiful

it( 'should detect a URL', () => {
const source = 'http://example.com/path';
expect( resemblesUrl( source ) ).to.equal( true );
} );

it( 'should detect a URL without protocol', () => {
const source = 'example.com';
expect( resemblesUrl( source ) ).to.equal( true );
} );

it( 'should detect a URL with a query string', () => {
const source = 'http://example.com/path?query=banana&query2=pineapple';
expect( resemblesUrl( source ) ).to.equal( true );
} );

it( 'should detect a URL with a short suffix', () => {
const source = 'http://example.cc';
expect( resemblesUrl( source ) ).to.equal( true );
} );

it( 'should return false with adjacent dots', () => {
const source = '..com';
expect( resemblesUrl( source ) ).to.equal( false );
} );

it( 'should return false with spaced dots', () => {
const source = '. . .com';
expect( resemblesUrl( source ) ).to.equal( false );
} );

it( 'should return false with a single dot', () => {
const source = '.';
expect( resemblesUrl( source ) ).to.equal( false );
} );

it( 'should return false if the string is not a URL', () => {
const source = 'exampledotcom';
expect( resemblesUrl( source ) ).to.equal( false );
} );
} );
6 changes: 4 additions & 2 deletions client/reader/follow-button/follow-sources.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ const exported = {
SEARCH_RESULTS: 'search-results',
READER_SUBSCRIPTIONS: 'reader-subscriptions',
READER_FEED_SEARCH: 'reader-feed-search-result',
COMBINED_CARD: 'reader-combined-card'
COMBINED_CARD: 'reader-combined-card',
READER_FOLLOWING_MANAGE_URL_INPUT: 'reader-following-manage-url-input',
};

export default exported;
Expand All @@ -16,5 +17,6 @@ export const {
SEARCH_RESULTS,
READER_SUBSCRIPTIONS,
READER_FEED_SEARCH,
COMBINED_CARD
COMBINED_CARD,
READER_FOLLOWING_MANAGE_URL_INPUT,
} = exported;
27 changes: 23 additions & 4 deletions client/reader/following-manage/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import FollowingManageSearchFeedsResults from './feed-search-results';
import MobileBackToSidebar from 'components/mobile-back-to-sidebar';
import { requestFeedSearch } from 'state/reader/feed-searches/actions';
import { addQueryArgs } from 'lib/url';
import FollowButton from 'reader/follow-button';
import { READER_FOLLOWING_MANAGE_URL_INPUT } from 'reader/follow-button/follow-sources';
import { resemblesUrl, addSchemeIfMissing, withoutHttp } from 'lib/url';

class FollowingManage extends Component {
static propTypes = {
Expand Down Expand Up @@ -94,7 +97,7 @@ class FollowingManage extends Component {
this.resizeSearchBox();

// this is a total hack. In React-Virtualized you need to tell a WindowScroller when the things
// above it has moved with a call to updatePosision(). Our issue is we don't have a good moment
// above it has moved with a call to updatePosition(). Our issue is we don't have a good moment
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't notice this before. thanks for the typo fix :)

// where we know that the content above the WindowScroller has settled down and so instead the solution
// here is to call updatePosition in a regular interval. the call takes about 0.1ms from empirical testing.
this.updatePosition = setInterval( () => {
Expand Down Expand Up @@ -129,14 +132,20 @@ class FollowingManage extends Component {
showMoreResults
} = this.props;
const searchPlaceholderText = translate( 'Search millions of sites' );
const showExistingSubscriptions = ! ( !! sitesQuery && showMoreResults );
const isSitesQueryUrl = resemblesUrl( sitesQuery );
let sitesQueryWithoutProtocol;
if ( isSitesQueryUrl ) {
sitesQueryWithoutProtocol = withoutHttp( sitesQuery );
}

return (
<ReaderMain className="following-manage">
<DocumentHead title={ 'Manage Following' } />
<MobileBackToSidebar>
<h1>{ translate( 'Manage Followed Sites' ) }</h1>
</MobileBackToSidebar>
{ ! searchResults && <QueryReaderFeedsSearch query={ sitesQuery } /> }
{ ! searchResults && ! isSitesQueryUrl && <QueryReaderFeedsSearch query={ sitesQuery } /> }
<h2 className="following-manage__header">{ translate( 'Follow Something New' ) }</h2>
<div ref={ this.handleStreamMounted } />
<div className="following-manage__fixed-area" ref={ this.handleSearchBoxMounted }>
Expand All @@ -153,8 +162,18 @@ class FollowingManage extends Component {
value={ sitesQuery }>
</SearchInput>
</CompactCard>

{ isSitesQueryUrl && (
<div className="following-manage__url-follow">
<FollowButton
followLabel={ translate( 'Follow %s', { args: sitesQueryWithoutProtocol } ) }
followingLabel={ translate( 'Following %s', { args: sitesQueryWithoutProtocol } ) }
siteUrl={ addSchemeIfMissing( sitesQuery, 'http' ) }
followSource={ READER_FOLLOWING_MANAGE_URL_INPUT } />
</div>
) }
</div>
{ !! sitesQuery && (
{ !! sitesQuery && ! isSitesQueryUrl && (
<FollowingManageSearchFeedsResults
searchResults={ searchResults }
showMoreResults={ showMoreResults }
Expand All @@ -165,7 +184,7 @@ class FollowingManage extends Component {
searchResultsCount={ searchResultsCount }
/>
) }
{ ! ( !! sitesQuery && showMoreResults ) && (
{ showExistingSubscriptions && (
<FollowingManageSubscriptions
width={ this.state.width }
query={ subsQuery }
Expand Down
31 changes: 31 additions & 0 deletions client/reader/following-manage/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,34 @@
border: 0;
}
}

.following-manage__url-follow {
border-bottom: 1px solid lighten( $gray, 30% );
padding: 13px 0 10px;
display: flex;
justify-content: center;

.follow-button {
.gridicon {
fill: $blue-medium;
}

.follow-button__label {
color: $blue-medium;

@include breakpoint ("<660px" ) {
display: inline;
}
}

&.is-following {
.gridicon {
fill: $alert-green;
}

.follow-button__label {
color: $alert-green;
}
}
}
}