feat: use human readable title instead of field title #377
Conversation
alexanderleegs
temporarily deployed
to
staging
GitHub Actions
Inactive
alexanderleegs
temporarily deployed
to
staging
GitHub Actions
Inactive
alexanderleegs
requested review from
harishv7,
dcshzj,
kishore03109 and
seaerchin
alexanderleegs
force-pushed
the
Feat/use-human-readable-title-instead-of-field-title-for-dgs-v2
branch
from
a915539 to
64fb8cf
Compare
assets/js/datagovsg-search.js
Outdated
| let formattedSearchField = searchField ? searchField.replace(" ", "_") : "" | ||
| if (!!searchField) { | ||
| // Datagov-v2 search | ||
| if (!!defaultField) { |
There was a problem hiding this comment.
why was this change required?
There was a problem hiding this comment.
It was previously incorrect and only applying for dgs-v2 layouts where someone had entered a search term - it was supposed to be for any dgs-v2 layout
assets/js/datagovsg-search.js
Outdated
| if (!columnMetadata) { | ||
| // V1 search, id and title are the same | ||
| columnMetadata = data.result.fields.map(field => ({ | ||
| id: field.id, | ||
| title: field.id, | ||
| })) | ||
| } | ||
| columnMetadata = columnMetadata || data.result.fields |
There was a problem hiding this comment.
this is confusing on 2 fronts:
columnMetadatais defined up above but we do the mapping here- why do the
mapifdata.result.fieldsis sufficient?
without context, i'd have thought data.result.fields alone would be enough. also, columnMeta exists only if there's a default field - i'm assuming without, we will use v1?
also, this is abit confusing here - we initialisecolumMetadata on resp.data - we seem to be assuming order of operations here (that the earlier initialisation will occur before request.done) but i'm not entirely sure this is true, especially in an async context.
can we just verify this behaviour or split up columnMetadata by the if/else?
There was a problem hiding this comment.
sorry the columnMetadata = columnMetadata || data.result.fields assignment isn't useful - have removed it in 282e2fa
The order of operations here holds because this is only executed after request.done - where the earlier assignment is done
| @@ -115,12 +134,12 @@ function databaseSearch(searchTerm, index, callback) { | |||
|
|
|||
| // The fieldArray is the array containing the field names in the data.gov.sg table | |||
| const removableFields = ["_id", "_full_count", "rank", `rank ${formattedSearchField}`] | |||
| fieldArray = remove(data.result.fields, removableFields); | |||
| fieldArray = remove(columnMetadata, removableFields); | |||
There was a problem hiding this comment.
pointing htis out because of sussy stuff wrt columMetadata above
This PR modifies the behaviour for dgs-v2 pages. For datasets on dgs v2, fields have both an internal name (e.g.
notification_no) and a human readable name (e.g.Notification number). We previously were using the internal name for the display of the table header, as well as the field dropdown, as the dgs-v1 datasets only contain an internal name. This PR changes that behaviour such that dgs-v2 pages will use the new human readable names for page display, while still sending the internal name to be used for search queries.Note that this means the field provided by the frontmatter should now be the human readable name. However, existing pages are unaffected in functionality - for the 2 sites currently making use of dgs-v2 pages, the deslugified versions of the internal representation and the field name are identical.