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: Added CoursewareSearchResults UI component #1224

Merged
merged 3 commits into from
Oct 27, 2023

Conversation

rijuma
Copy link
Member

@rijuma rijuma commented Oct 25, 2023

Description

Ticket: KBK-44 🔒

We need a Search Results UI component 🔒 to show the search components and the results.

UI changes

Showing results

320px wide 1024px wide
image image

No results

320px wide 1024px wide
image image

Testing Instructions:

This shows mocked results, so in order to see the variations:

  • Follow the instructions to enable the waffle flags as indicated in Added waffle flag state for Courseware Search #1199.
  • Searching for lorem ipsum will show the "no results" variant.
  • Searching for anything other than lorem ipsum will show a fixed number of results.
  • Performing an empty search will clear the results.

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (7c92110) 88.19% compared to head (6852d4b) 88.18%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1224      +/-   ##
==========================================
- Coverage   88.19%   88.18%   -0.02%     
==========================================
  Files         271      273       +2     
  Lines        4591     4620      +29     
  Branches     1151     1164      +13     
==========================================
+ Hits         4049     4074      +25     
- Misses        528      531       +3     
- Partials       14       15       +1     
Files Coverage Δ
...home/courseware-search/CoursewareSearchResults.jsx 100.00% <100.00%> (ø)
src/course-home/courseware-search/messages.js 100.00% <ø> (ø)
...-home/courseware-search/test-data/mockedResults.js 100.00% <100.00%> (ø)
...course-home/courseware-search/CoursewareSearch.jsx 64.70% <33.33%> (-35.30%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rijuma rijuma force-pushed the rijuma/courseware-search-results-ui branch from 71cc670 to 7712a78 Compare October 25, 2023 16:00
@rijuma
Copy link
Member Author

rijuma commented Oct 25, 2023

This is a UI component with mocked behavior for testing. I don't think this needs to have coverage atm. It will be wrapped up in an implementation ticket.

@rijuma rijuma marked this pull request as ready for review October 25, 2023 16:01
Copy link
Contributor

@schenedx schenedx left a comment

Choose a reason for hiding this comment

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

👍

@rijuma rijuma merged commit c257048 into master Oct 27, 2023
3 of 5 checks passed
@rijuma rijuma deleted the rijuma/courseware-search-results-ui branch October 27, 2023 13:57
CefBoud pushed a commit to open-craft/frontend-app-learning that referenced this pull request Nov 5, 2023
* feat: Added CoursewareSearchResults UI component

* fix: Added conditional for undefined case instead of null

* fix: Updated code to avoid mutation
CefBoud pushed a commit to open-craft/frontend-app-learning that referenced this pull request Nov 6, 2023
* feat: Added CoursewareSearchResults UI component

* fix: Added conditional for undefined case instead of null

* fix: Updated code to avoid mutation
CefBoud pushed a commit to open-craft/frontend-app-learning that referenced this pull request Nov 6, 2023
* feat: Added CoursewareSearchResults UI component

* fix: Added conditional for undefined case instead of null

* fix: Updated code to avoid mutation
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.

3 participants