Skip to content
This repository has been archived by the owner on Jun 12, 2024. It is now read-only.

[search] Create Search component #121

Merged
merged 48 commits into from
May 10, 2019
Merged

[search] Create Search component #121

merged 48 commits into from
May 10, 2019

Conversation

katydecorah
Copy link
Contributor

@katydecorah katydecorah commented Apr 10, 2019

This PR works to add a new Search component that will add an input hooked up to Swiftype. Results are displayed in a popover under the input.

To test:

  1. npm start
  2. Navigate to http://192.168.7.223:9966/Search

The test has two examples - basic search and in context of where I think it could go on each site (on the right-end of the top bar sticker).

QA Checklist

Responsive

  • Check on mobile-size viewport.
  • Check on tablet-size viewport.
  • Check on laptop-size viewport.
  • Check on big-monitor-size viewport.

Cross-browser

  • Check in Chrome.
  • Check in Firefox.
  • Check in Safari.
  • Check in Edge.
  • Check in IE11.
  • Check in mobile Safari.
  • Check in Android's Chrome.

@katydecorah katydecorah changed the title 🚧 Creaate Search component 🚧 Create Search component Apr 10, 2019
Katy DeCorah added 5 commits May 9, 2019 19:27
* master:
  0.10.0
  fix prerelease order in sortVersion helper function (#124)
@katydecorah katydecorah marked this pull request as ready for review May 10, 2019 13:18
@katydecorah katydecorah changed the title 🚧 Create Search component Create Search component May 10, 2019
@katydecorah
Copy link
Contributor Author

@colleenmcginnis this is ready for review!

Note: search doesn't appear at all in IE11:

image

I haven't pinpointed the exact point of failure, but I feel ok with punting on getting the search to work in IE11 since it gracefully degrades (to no search) and doesn't break the layout.

## Master

- Add `Search` component
- 🚨 [Breaking change] Remove `txt-bold` and `txt-s` from `LevelIndicator` component to make it more flexible.
Copy link
Contributor Author

@katydecorah katydecorah May 10, 2019

Choose a reason for hiding this comment

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

💡When we update dr-ui in other repos using LevelIndicator we'll want to wrap the component in a span or div with these classes where needed.

</ul>
) : (
<div className="py12 px12">
Sorry, we didn't find anything.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy suggestions welcome!

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 @HeyStenson any thoughts here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something like: Hmmm, we didn't find anything. Reword your search, or contact Support. (with a link to the support page).

❓❓❓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I love that @HeyStenson! I'll add it 🙇‍♀

@@ -0,0 +1,90 @@
import React from 'react';
import PropTypes from 'prop-types';
import Downshift from 'downshift';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

💡Downshift makes our search input and and results navigable by keyboard!

Copy link
Contributor

Choose a reason for hiding this comment

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

ooooooo!

)}
{subsite && subsite !== title && subsite !== site ? (
<span>
{subsite.replace(/\sfor\s(iOS|Android|Vision)/, '')}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

💡This temporarily removes the "for Android" (or iOS or Vision) from the subsite name. We should eventually update the docs-page-shell to strip this out but this will fix it on the frontend to give us cleaner looking results.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call! Much cleaner. Should we do the same for Unity?

image

Copy link
Contributor

@colleenmcginnis colleenmcginnis left a comment

Choose a reason for hiding this comment

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

@katydecorah this is a work of art.

I think there may be some design considerations we'll need to revisit when we roll this out to the wider collection of docs repos (for example, the width available in top bar when we have a few tabs), but I think this iteration is ready to test on the help page!!

@katydecorah katydecorah merged commit 6a45611 into master May 10, 2019
@katydecorah katydecorah deleted the search branch May 10, 2019 20:42
katydecorah pushed a commit that referenced this pull request May 14, 2019
* master:
  0.11.1
  wrap LevelIndicator in txt-s txt-bold (#126)
  0.11.0
  Prepare 0.11.0
  Create Search component (#121)
  0.10.0
  fix prerelease order in sortVersion helper function (#124)
@katydecorah katydecorah changed the title Create Search component [search] Create Search component Jul 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants