Skip to content

Global search bar (core)#1779

Merged
joelit merged 46 commits intomainfrom
issue1489-global-search-bar
Nov 6, 2025
Merged

Global search bar (core)#1779
joelit merged 46 commits intomainfrom
issue1489-global-search-bar

Conversation

@joelit
Copy link
Contributor

@joelit joelit commented Mar 18, 2025

Link to relevant issue(s), if any

Description of the changes in this PR

Known problems or uncertainties in this PR

Not implemented in this PR:

  • Translated messages
    • Placeholder texts for the language and vocabulary selectors, and for the search field
    • Number of hits in the dropdown (do we need this?)
    • 'No hits' message
  • Extra concept information the dropdown list
    • SkosXL labels
    • Concept type translations
  • Accessability
    • Keyboard use
    • Better semantic structure for the language and vocabulary selectors (screen reader tests)
  • General header bar functionlity
    • Hide the search bar -button
    • 'Search tips' menu

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@joelit joelit added this to the 3.0 milestone Mar 18, 2025
@joelit joelit self-assigned this Mar 18, 2025
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
66.9% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@joelit joelit moved this to In progress in Skosmos 3.x Backlog Mar 18, 2025
@osma osma modified the milestones: 3.0-alpha.1, 3.0-alpha.2 Mar 20, 2025
@joelit joelit moved this from In progress to Current sprint backlog in Skosmos 3.x Backlog May 13, 2025
@joelit joelit moved this from Current sprint backlog to In progress in Skosmos 3.x Backlog May 15, 2025
@joelit joelit force-pushed the issue1489-global-search-bar branch from c152756 to e09613c Compare May 15, 2025 12:13
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
67.6% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@osma osma modified the milestones: 3.0-alpha.2, 3.0-beta.1 May 26, 2025
@joelit joelit force-pushed the issue1489-global-search-bar branch 2 times, most recently from 69bacea to 4b2eeb7 Compare August 21, 2025 10:47
@joelit joelit marked this pull request as ready for review August 21, 2025 11:39
@joelit joelit requested a review from UnniKohonen August 21, 2025 11:39
Copy link
Contributor

@UnniKohonen UnniKohonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General layout/usability notes:

  • All vocabularies should be the first option in the vocab dropdown menu
  • There should maybe be some padding/margins inside the vocab dropdown menu
  • The button closing the search bar is not implemented
  • Checkboxes should have custom styling
  • Checking/unchecking vocabs is annoying because the dropdown moves
  • There is some lag in the animation for me when opening/closing the menu

@joelit joelit moved this from In progress to Reviewed - further actions needed in Skosmos 3.x Backlog Aug 26, 2025
@joelit joelit moved this from Reviewed - further actions needed to Needs review in Skosmos 3.x Backlog Aug 27, 2025
Copy link
Contributor

@UnniKohonen UnniKohonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new vocab list looks great! I forgot to mention this in the last review but the vocab list is missing the option for all vocabularies, is it supposed to be included in Skosmos 3.0/this PR?

Some small nitpicks about the layout:

  • In the layout guide, the unchecked checkboxes have a light background and a dark border (but the guide is for Finto and not Skomos so it's probably OK)
  • The magnifying glass button is not a square

@osma osma moved this from Needs review to Reviewed - further actions needed in Skosmos 3.x Backlog Aug 28, 2025
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
16.7% Duplication on New Code (required ≤ 3%)
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@joelit joelit moved this from Reviewed - further actions needed to In progress in Skosmos 3.x Backlog Oct 29, 2025
@osma osma modified the milestones: 3.0-beta.1, 3.0-beta.2 Oct 30, 2025
@joelit joelit requested a review from osma November 5, 2025 10:39
@joelit
Copy link
Contributor Author

joelit commented Nov 5, 2025

SonarQube is complaining about the repetitive nature of the two dropdown menus in the search bar, wishing for a generic parameterized implementation that renders the different dropdown menus. In this case where we should ever have just the two menus for vocab and language, I am not sure getting rid of the repetitive code would improve the readability or the maintainability of the code.

@osma osma moved this from In progress to Needs review in Skosmos 3.x Backlog Nov 5, 2025
@osma osma changed the title Issue1489 global search bar Global search bar (core) Nov 5, 2025
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. Obviously some functionality is still missing (e.g. proper i18n) but it's important to get the core merged so that other work can continue.

The only thing I would change before merging this PR is the handling of URL parameters: handle the parameters for the search result page separately from the REST API search method parameters. Suggestions in the comments.

Also, I suggested adding a couple of trans filters in the Twig templates, so we don't forget that they need to be translated. You can simply click to accept my suggestions if those are OK.

Other things I noticed that probably need to be considered later:

  1. Handle translations for the UI messages, both in JS and in Twig templates.
  2. I tested what happens if you don't select any vocabularies. Then the vocab parameter is left empty and search is performed over all vocabularies. Not a bad outcome, but maybe a bit surprising... Another option would be to not perform a search until the user has made a selection.
  3. If I don't select a language, the clang parameter will be set to null and the search always returns an empty result. Again, I think it would be better to not perform a search until the user has chosen a language.
  4. On the search result page, the global search bar is available but initially hidden. I think it would be better to always show it on the search result page. Also, it should remember the selections and the keyword.
  5. Minor detail: When selecting multiple vocabularies, they are listed as e.g. "YSO YKL JUHO Metatietosanasto", which may be a bit confusing; I suggest separating multiple values with commas instead, like in Skosmos 2.

@joelit
Copy link
Contributor Author

joelit commented Nov 6, 2025

The requested changes were applied. I'll make sure the related epic contains the updated specifications, or the request for discussion thereof. I ended up splitting the language updating mechanism from the language parsing mechanism for ease of maintenance.

@joelit joelit moved this from Needs review to Reviewed - further actions needed in Skosmos 3.x Backlog Nov 6, 2025
@joelit joelit requested a review from osma November 6, 2025 11:24
@joelit joelit moved this from Reviewed - further actions needed to Needs review in Skosmos 3.x Backlog Nov 6, 2025
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I found a couple of minor CSS variable mishaps that perhaps could be fixed. Also, I'd like to avoid using Finnish language messages in the code, so I suggested changing the base messages to English for the benefit of non-Finnish testers of 3.0beta2. (If you change those, Cypress tests will need to be adjusted accordingly as well.)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
10.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@joelit joelit merged commit ea631f9 into main Nov 6, 2025
13 of 14 checks passed
@github-project-automation github-project-automation bot moved this from Needs review to Issue/PR closed in Skosmos 3.x Backlog Nov 6, 2025
@joelit joelit deleted the issue1489-global-search-bar branch November 6, 2025 12:13
@joelit joelit moved this from Issue/PR closed to Done (verified in test.dev.finto.fi, set Milestone 3.0 for both issue & PR, update docs) in Skosmos 3.x Backlog Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants