Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting the language in the search bar #1629

Merged
merged 15 commits into from
Apr 25, 2024
Merged

Conversation

joelit
Copy link
Contributor

@joelit joelit commented Apr 24, 2024

Link to relevant issue(s), if any

#1514

Description of the changes in this PR

  • Rendering the search results when "All languages" is chosen
  • Choosing the correct search language
    • First from the url clang parameter
    • Second from cookie
    • If all else fails, from the SKOSMOS object

Known problems or uncertainties in this PR

Needs more cypress tests

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 requested a review from UnniKohonen April 24, 2024 10:03
@joelit joelit self-assigned this Apr 24, 2024
@joelit joelit added this to the 3.0 milestone Apr 24, 2024
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.

Setting the language on a vocab home page and then on a concept page in the same vocab will set multiple SKOSMOS_SEARCH_LANG cookies with different paths. I'm not sure what effect this has but maybe the cookies should be cleared before setting a new one.

Otherwise everything seems to be working correctly.

@joelit joelit marked this pull request as ready for review April 24, 2024 13:08
@joelit joelit requested a review from UnniKohonen April 25, 2024 08:03
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.

When the anylang URL parameter is set, the value of the cookie is set to null. This should be fixed.

There is also a cypress test that fails but I don't know if the problem originates from this PR.

Otherwise everything looks good

resource/js/vocab-search.js Outdated Show resolved Hide resolved
resource/js/vocab-search.js Show resolved Hide resolved
Copy link

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@joelit joelit requested a review from UnniKohonen April 25, 2024 10:33
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.

LGTM!

@joelit joelit merged commit 27ce825 into main Apr 25, 2024
10 checks passed
@joelit joelit deleted the issue1514-searchBarLanguage branch April 25, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (verified in test.dev.finto.fi, set Milestone 3.0 for both issue & PR)
Development

Successfully merging this pull request may close these issues.

2 participants