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

[search] Introduce search modal #133

Merged
merged 31 commits into from
May 30, 2019
Merged

[search] Introduce search modal #133

merged 31 commits into from
May 30, 2019

Conversation

colleenmcginnis
Copy link
Contributor

@katydecorah I had (maybe too much) fun exploring some possibilities here. This approach builds on your PR (#130) using a modified/simplified version of the mr-ui Modal component with a search bar in it on wide screens and uses the regular search bar below tabs on narrow screens.

Take a look and let me know what you think! If you think this is too complex for this stage of testing we can revisit this idea later. I think pulling this out of the top bar could also give us more room to add filters or other options in the future.

Wide screens:
Screen Shot 2019-05-25 at 1 13 07 PM

Narrow screens:
Screen Shot 2019-05-25 at 1 13 26 PM

@katydecorah
Copy link
Contributor

katydecorah commented May 29, 2019

@colleenmcginnis this is amazing!!!!!

I hopped in with some design and accessibility tweaks. I also added two props disableModal and narrow. Here are some screenshots:

Default component

Added a "Search" label for additional context and light styling

image

Dark vs light

image

disableModal option

For pages like 404 and docs landing page when we have lots of space.

image

narrow option

To remove label from search button for sites like /mapbox-gl-js

image

Modal

Made icon and text sizes a little bigger:

image

To do

Copy link
Contributor

@katydecorah katydecorah left a comment

Choose a reason for hiding this comment

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

@colleenmcginnis want to give this a second look?

@colleenmcginnis
Copy link
Contributor Author

✅ The updates look beautiful, @katydecorah!

I did notice the button extending outside the lines just before hitting the breakpoint in this example, but I guess this would be solved by using the narrow option in a case with this many tab items?

image

@katydecorah
Copy link
Contributor

@colleenmcginnis thanks for flagging! I think we should be ok. Looking at /mapbox-gl-js

image

The navigation folds to two lines and the Search component should follow suit.

The test case isn't as robust as the actual application, but I'll look out for this during the install process.

@katydecorah
Copy link
Contributor

katydecorah commented May 30, 2019

Gonna QA before the merge:

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.

Katy DeCorah added 3 commits May 30, 2019 16:10
* master:
  prepare 0.13.0
  adjust version-sort for XX.XX.XX format (#132)
@katydecorah katydecorah changed the title [search] Introduce search modal (possibly) [search] Introduce search modal May 30, 2019
@katydecorah katydecorah changed the base branch from search-collapse to master May 30, 2019 20:18
@katydecorah katydecorah merged commit eec4def into master May 30, 2019
@katydecorah katydecorah deleted the search-modal-possibly branch May 30, 2019 20:31
katydecorah pushed a commit that referenced this pull request May 30, 2019
* master:
  [search] Introduce search modal (#133)
katydecorah pushed a commit that referenced this pull request Jun 5, 2019
* master:
  add snapshot tests (#136)
  0.15.1
  Prepare 0.15.1
  0.15.0
  Prepare 0.15.0
  [search] adds segment tracking for queries and clicks (#140)
  [search] add ability to filter results (#138)
  increase modal zindex to prevent collision with topbar sticker (#139)
  0.14.0
  Prepare 0.14.0
  [PageLayout/TopbarSticker] increase zIndex (#134)
  [search] Introduce search modal (#133)
  prepare 0.13.0
  adjust version-sort for XX.XX.XX format (#132)
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.

2 participants