Global Search twig-template #1758
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1758 +/- ##
=========================================
Coverage 70.94% 70.94%
Complexity 1650 1650
=========================================
Files 33 33
Lines 4330 4330
=========================================
Hits 3072 3072
Misses 1258 1258 ☔ View full report in Codecov by Sentry. |
joelit
force-pushed
the
issue1745-global-search-twig
branch
from
0cb3322 to
a308ba2
Compare
|
|
Rebased the branch to main in order to allow automatic merging |
There was a problem hiding this comment.
I gave a few suggestions for minor code adjustments.
Looking at the search result page, I see several layout issues. The spacing between the icons and the text is sometimes nonexistent, and there are sometimes extra spaces before commas:
But this is not consistent as sometimes the result items are rendered just fine:
| @@ -91,12 +91,13 @@ body { | |||
| margin: 0 5px; | |||
| } | |||
|
|
|||
| #main-container.vocab-search .fa-arrow-right { | |||
| #main-container.searchpage .fa-arrow-right, #main-container.global-search .fa-arrow-right { | |||
There was a problem hiding this comment.
I think we just got rid of .searchpage, now it's coming back again? Change it to .vocab-search
| color: var(--vocab-text); | ||
| font-size: 12px; | ||
| } | ||
|
|
||
| #main-container.vocab-search .list-group .fa-solid, #main-container.vocab-search .list-group .fa-regular { | ||
| #main-container.searchpage .list-group .fa-solid, #main-container.searchpage .list-group .fa-regular, |
There was a problem hiding this comment.
Ditto, change .searchpage to .vocab-search
| @@ -0,0 +1,21 @@ | |||
| {% set pageType = 'global-search' %} | |||
| {% extends "base-template.twig" %} | |||
| {% block title %}'{{ term }}' - {{ "Search results" | trans }} - {{ GlobalConfig.serviceName }}{% endblock %} | |||
There was a problem hiding this comment.
According to the current SEO spec for page titles and descriptions, the middle part "Search results" should be dropped. So the title should include just the search term and the service name.
| it('Contains title and title metadata', () => { | ||
| cy.visit(`/en/search?clang=en&q=${term}`) | ||
|
|
||
| const expectedTitle = "'bass' - Search results - Skosmos being tested" |
There was a problem hiding this comment.
This of course needs to be changed as well if you change the title in the template above, as suggested.
|
OK, the inconsistency of the search results for search result property label spacing and property value spacing is pretty weird, and probably has further implications for the consistency of the twig template. I'll track this down to the root cause. |
Reasons for creating this PR
Link to relevant issue(s), if any
Description of the changes in this PR
[Skosmos-home]/en/search?clang=en&q=kissaKnown problems or uncertainties in this PR
Checklist
.sr-onlyclass, color contrast)