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

feat: front-end implementation for search phase 1 #227

Merged
merged 5 commits into from
Jul 20, 2020

Conversation

JasonChong96
Copy link
Contributor

@JasonChong96 JasonChong96 commented Jun 29, 2020

Problem

With the back-end implementation done, the public can now search for links through API calls. This PR implements the front-end UI for interacting with the search feature

Solution

Features:

  • Integrated home page that includes a search bar
  • This integrated page can be toggled under src/client/util/config.ts. If IS_SEARCH_HIDDEN is set to true, the only way to reach the search page is through a direct URL.
  • IS_SEARCH_HIDDEN is set to true in this PR.
  • A dedicated search page
  • Search parameters are controlled by URL parameters
  • Search input and results have a maximum width to prevent weird scaling on large viewports
  • Changes in query are debounced such that changes are only applied to the url and api call 500ms after users stop typing

Improvements:

  • SearchResultsSortOrder has been moved to src/shared/ as it is used by both the back and front-end code
  • PaginationActionComponent is now in the shared widgets folder due to it being used in both User page and Search page
  • SortPanel has been extracted as a widget for the same reason
  • CollapsingPanel has been extracted for the same reason
  • CloseIcon, SearchIcon and SearchSortIcon are now resizable on IE11 and modern browsers
  • CloseIcons should be closer in size to the design in all pages
  • Fix typo in component name LandingGraphicSliver.
  • BaseLayoutHeader should use icons in mobile view instead of text

Before & After Screenshots

AFTER:
Integrated landing page (Desktop)
Integrated landing page (Mobile)
Search (Desktop)
Search (Mobile)
Search Info (Mobile)
Search Sort (Mobile)
Search Sort (Desktop)
Empty search (Desktop)
Empty search (Mobile)

Additional notes

  • No code sharing between the old and new landing page graphics to make it easy to delete the old one once search is officially launched and stable.
  • IS_SEARCH_HIDDEN is not an env var as it should be shown on dev/testing environments whenever it is enabled on production

TODO:

  • Implement integrated home page
  • Implement desktop UI for search
  • Implement mobile UI for search
  • Use URL parameters as the single source of truth for search parameters
  • Implement additional panels needed for mobile UI
  • Disable transition page when accessing a link from GoSearch
  • Pending design tweaks, empty result placeholder
  • Add screenshots to this PR description

@JasonChong96 JasonChong96 marked this pull request as ready for review July 8, 2020 18:35
@JasonChong96 JasonChong96 requested a review from liangyuanruo July 8, 2020 18:35
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

Tested the app and it seems OK. I would also suggest some UI tweaks, which you may wish to discuss with the designers:

  • Removing the "sign in" button on search pages so that members of the public don't try and login, which will trigger 401 Unauthorized alarms
  • Removing the "GoSearch" button when IS_SEARCH_HIDDEN is set to false, as the search bar is already prominent
  • Adjust the CSS for the bottom banner so that it remains flush to the bottom on the search page

@JasonChong96
Copy link
Contributor Author

Tested the app and it seems OK. I would also suggest some UI tweaks, which you may wish to discuss with the designers:

  • Removing the "sign in" button on search pages so that members of the public don't try and login, which will trigger 401 Unauthorized alarms
  • Removing the "GoSearch" button when IS_SEARCH_HIDDEN is set to false, as the search bar is already prominent
  • Adjust the CSS for the bottom banner so that it remains flush to the bottom on the search page

Resolved first and last issues. Iirc, the second was discussed with the designers before and it was a deliberate choice

@liangyuanruo
Copy link
Contributor

Resolved first and last issues. Iirc, the second was discussed with the designers before and it was a deliberate choice

OK noted, @pearlyong FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants