Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Feature #160722817 – Add search examples below the search box #8

Merged
merged 7 commits into from
Nov 23, 2018

Conversation

alfonsomunozpomer
Copy link
Member

If we need more flexibility we can let the client insert a full React component, but for the current story this is enough, I guess.

@coveralls
Copy link

coveralls commented Nov 22, 2018

Pull Request Test Coverage Report for Build 78

  • 5 of 5 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 92.708%

Totals Coverage Status
Change from base Build 65: 0.4%
Covered Lines: 49
Relevant Lines: 54

💛 - Coveralls

links.map(
(link, index) =>
<span key={index}>
{index === 0 && ` `}<a href={link.url}>{link.text}</a>{index < links.length - 1 && `, `}
Copy link
Contributor

Choose a reason for hiding this comment

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

The last link should not be followed by a comma I believe.

Copy link
Member Author

Choose a reason for hiding this comment

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

It won’t be, notice that the right hand side is links.length -1 instead of links.length.

Copy link
Contributor

Choose a reason for hiding this comment

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

The comma is shown on the SC homepage actually.

screen shot 2018-11-23 at 11 52 38

Copy link
Contributor

Choose a reason for hiding this comment

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

It's weird since I try the same test searchExamples data in the local search form branch, which works fine. However, it appears differently on the atlas branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yes, that’s because the bundles were from the penultimate release. If you generate the bundles yourself and try it out it’ll be ok.

Copy link
Contributor

@lingyun1010 lingyun1010 left a comment

Choose a reason for hiding this comment

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

All looks good. I would suggest adding a searchExamples prop in one of the html demos.

@alfonsomunozpomer alfonsomunozpomer merged commit 40b55f6 into master Nov 23, 2018
@alfonsomunozpomer alfonsomunozpomer deleted the feature/160722817-add-search-examples branch November 23, 2018 15:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants