Feature/results model #1307
Feature/results model #1307
Conversation
lukavdplas
added
code quality
lukavdplas
force-pushed
the
feature/results-model
branch
from
728f4f8 to
e958ebd
Compare
frontend/src/app/document/document-popup/document-popup.component.ts
Outdated
Show resolved
Hide resolved
| private focusPosition(position: number) { | ||
| const index = _.clamp(position - 1, 0, this.documents.length - 1); | ||
| this.focus(this.documents[index]); | ||
| } | ||
| } |
There was a problem hiding this comment.
This method could use a (short) docstring
| import { Results } from './results'; | ||
| import { DocumentPage } from './document-page'; | ||
|
|
||
| export const RESULTS_PER_PAGE = 20; |
There was a problem hiding this comment.
How often do we expect to change this value? Could it be variable for different flavours of I-Analyzer? If so, this may belong in environment.
There was a problem hiding this comment.
Hm, good one. I think the more conventional way of handling it is to put a control for this next to pagination, so let users decide. But if I was going to work on expanding the results page right now, I would prioritise #1011, #1012, #729, or #996 - a size control seems low-priority to me. It may bring more clutter than value.
As for adding it to environment settings: that is technically trivial, but then I don't think have a reason to change this between our existing environments. Perhaps we could think more long-term about what we envision for different I-analyzer instances.
There was a problem hiding this comment.
Stray thought on this: size=20 is a bit trivial, since that is the elasticsearch default. Perhaps if we did not specify this, it could also be configured in the elasticsearch settings? Which would also make it environment-configurable.
| return {}; | ||
| } | ||
|
|
||
| setParameters(newValues: Partial<Parameters>) { |
There was a problem hiding this comment.
Same applies for the methods, take assignOnQueryUpdate as a good example
Frontend refactoring:
PageResultsmodel to represent search results.PageResultsextends aResultsmodel that is intended to be used in some other contexts in the future (e.g. aggregation requests)SearchResultsComponentDocumentPopupComponentfrom the search results componentContent changes:
close #1162