Skip to content
This repository has been archived by the owner. It is now read-only.

feat(highlights): Show more diverse highlights by limiting to one per domain #3398

Merged
merged 1 commit into from Sep 9, 2017

Conversation

@Mardak
Copy link
Member

@Mardak Mardak commented Sep 9, 2017

Fix #3390. r?@rlr Did some refactoring / clean up too.

Before (team):
screen shot 2017-09-09 at 11 08 42 am

After (only aaron):
screen shot 2017-09-09 at 11 10 25 am

@Mardak Mardak requested a review from rlr Sep 9, 2017
@rlr
rlr approved these changes Sep 9, 2017
Copy link
Contributor

@rlr rlr left a comment

LGTM. just one tiny nit. r+

}
// Request more than the expected length to allow for items being removed by
// deduping against Top Sites or multiple history from the same domain, etc.
const numItems = HIGHLIGHTS_MAX_LENGTH * 5 + TOP_SITES_SHOWMORE_LENGTH;

This comment has been minimized.

@rlr

rlr Sep 9, 2017
Contributor

no need to do this math on every fetch #nano-optimization

This comment has been minimized.

@Mardak

Mardak Sep 9, 2017
Author Member

Moved to top as MANY_EXTRA_LENGTH

@Mardak Mardak force-pushed the Mardak:gh3390-one branch from e4872e3 to 544f66f Sep 9, 2017
@Mardak Mardak merged commit d97dd69 into mozilla:master Sep 9, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Mardak Mardak deleted the Mardak:gh3390-one branch Sep 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants