Courses Smart Search Bar

[suhaibabsi-inst]

refs: MBL-17907,MBL-17906,MBL-17905,MBL-17904,MBL-17903,MBL-17902
affects: Student
release note: Courses Smart Search Experience for Student

Related PR(s):
#2967

Test Plan

• Make sure to activate "Smart Search" on account & course level. See here.
• Login to Student app.
• Go to courses, open one of the courses.
• You should expect a search icon shown on the top-right button. Follow on the experience.
  • Showing search results
  • Selecting Filter
  • Showing grouped results
  • Highlight visited results

Video Record

Simulator.Screen.Recording.-.iPhone.14.-.2024-10-21.at.16.23.10.mp4

Checklist

• Follow-up e2e test ticket created
• A11y checked
• Tested on phone
• Tested on tablet
• Tested in dark mode
• Tested in light mode
• Approve from product

[inst-danger]

Student Build QR Code:

[inst-danger]

	Warnings
⚠️	One or more files are below the minimum test coverage 50%

Coverage	New %	Master %	Delta
Canvas iOS	90.03%	90.17%	-0.13%
Core/Core/Planner/CalendarMain/ViewModel/PlannerViewModel.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/Model/CalendarWeek.swift	0%	0%	0%
Core/Core/Extensions/CGSizeExtensions.swift	0%	--	--
Core/Core/Planner/CalendarMain/View/Calendar/CalendarCardInteractionState.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/Model/PlannablesInteractor.swift	0%	0%	0%
Core/Core/Search/Model/SearchSupportButtonModel.swift	0%	--	--
Core/Core/Planner/CalendarMain/View/Calendar/ViewPreferences.swift	0%	0%	0%
Core/Core/Planner/CalendarMain/Model/CalendarDay.swift	0%	0%	0%
Core/Core/Courses/SmartSearch/View/CourseSmartSearchViewsProvider.swift	0%	--	--
Core/Core/Courses/SmartSearch/Model/CourseSmartSearchViewAttributes.swift	30.77%	--	--

Generated by 🚫 dangerJS against f45221d

[rh12]

Thanks for the great work!
I made a first round of review/testing and collected these findings so far:

1. I would suggest to use the filterSolid instead of filterActiveSolid, like in Grade or Assignment filtering. But we should align with Marci first.
2. I think the filter icon should not turn to filtered state when only the "Sort by" is changed.
3. Should the course header image disappear when the SmartSearch icon is tapped?
4. It's probably not in our hands but searching for random characters (like Sdhaslfjabhldfbany) still provides results
   
5. IMHO the filter screen should apply the changes on swipe-to-dismiss, just like when tapping Done (like with Assignment filters)
6. According to figma, tapping back from a result should fade out the selected state of the cell (like in standard iOS apps do), not keep the selection (please also see my comment on ContextButton).

[suhaibabsi-inst]

Thanks for the great work! I made a first round of review/testing and collected these findings so far:

1. I would suggest to use the filterSolid instead of filterActiveSolid, like in Grade or Assignment filtering. But we should align with Marci first.
2. I think the filter icon should not turn to filtered state when only the "Sort by" is changed.
3. Should the course header image disappear when the SmartSearch icon is tapped?
4. It's probably not in our hands but searching for random characters (like Sdhaslfjabhldfbany) still provides results
   
5. IMHO the filter screen should apply the changes on swipe-to-dismiss, just like when tapping Done (like with Assignment filters)
6. According to figma, tapping back from a result should fade out the selected state of the cell (like in standard iOS apps do), not keep the selection (please also see my comment on ContextButton).

Thanks @rh12 , I think we should be discussing most of those points with Marci & Judit.
For point 5, will be handled as you suggested.
For point 4, we also investigating another issue related to it, which is the mismatch of results between Web and Mobile for the same course and search keywords. We might need more help from BE on that, but I am going to capture that in some sort of technical way.

[rh12]

This is my final round of code review. There are a lot of small comments, sorry about that. They mostly aim clarity and/or consistency.

Some general notes:

• please add doc-comments to the types, especially in Search/Model folder
• if you add non-private extensions or helpers, please put them in some common space, unless they are specific (both in naming and in function) to some feature
• please use trailing closure syntax where applicable
• if you add useful common components, it helps discoverability if you call them out in the PR description (doc-comments help those as well)
• it also helps understanding a large and complex PR if some of it's main concepts or reasons are summarized in the PR description

Regarding accessibility, please make sure that

• all new screens (except the Search Bar) are resizing dynamically when text size is changed
• all new screens and the Search Bar are read out properly

I plan to do a QA round after changes are finalized.

[vargaat]

Nice work on getting all this together! I submit some comments while you work on others. My general feeling is that the code became pretty complex with all those protocols and generics compared to the task it does. Without comments and (for an outsider) non-meaningful names makes it hard to understand. I'd be happy if we could look for a way to eliminate some of the complexity and generalization.

[vargaat]

Interactors should go to the model layer.

[vargaat]

In general I've had a hard time time understanding how these entities work together. One thing we could improve here is to add a better name. The CourseSmartSearch name tells me that this entity encapsulates an actual search operation, maybe holds the search text, results and so on. Turns out it's just mainly a descriptor about how the view should look.

[vargaat]

This file seems to be a mix of view/viewmodel and interactor/business logic related code. I'd definitely move all UI related code to an upper level.

[suhaibabsi-inst]

This is to be determined later. As it might be unnecessary to use paged fetching for search results, while there is no intention to show results with relevance >= 50.