Gh-524-search-page #587
Gh-524-search-page #587
Conversation
OButlerOcc
force-pushed
the
feature/gh-524-search-page
branch
from
441dcb2
to
56b2e15
Compare
There was a problem hiding this comment.
Overall this is very good @OButlerOcc . It works as expected, and your inclusion of test cases are well thought out.
I have left a few minor things for you cleanup before we'll consider this finished, if you see to them please.
src/app/catalogue-search/catalogue-search-form/catalogue-search-form.component.html
Show resolved
Hide resolved
src/app/catalogue-search/catalogue-search-form/catalogue-search-form.component.html
Outdated
Show resolved
Hide resolved
src/app/catalogue-search/catalogue-search-form/catalogue-search-form.component.html
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| reset() { | ||
| this.formGroup.controls.context.setValue(null); |
There was a problem hiding this comment.
I've noticed that clicking the "Reset" button doesn't actually clear the context selection tree control. Because that is a custom control, perhaps you need to manually reset the inputs for it.
There was a problem hiding this comment.
Did not fix for now, using the MDMTreeSearchComponent as a view child binds the components together and want to avoid modifying MDMTreeSearchComponent if possible. Could also use the broadcast service but that seems out of scope for a small change
src/app/catalogue-search/catalogue-search-form/catalogue-search-form.component.scss
Outdated
Show resolved
Hide resolved
src/app/catalogue-search/catalogue-search/catalogue-search.component.scss
Outdated
Show resolved
Hide resolved
src/app/catalogue-search/catalogue-search-listing/catalogue-search-listing.component.ts
Outdated
Show resolved
Hide resolved
src/app/catalogue-search/catalogue-search/catalogue-search.component.spec.ts
Show resolved
Hide resolved
Co-authored-by: Peter Monks <Peter.Monks@oxfordcc.co.uk>
Rewrote search page.
Search page routes to search listing with url params
Tests for each part of the form