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

SEO: descriptions on landing, about and feedback pages #1647

Merged
merged 3 commits into from
Aug 15, 2024

Conversation

osma
Copy link
Member

@osma osma commented Aug 14, 2024

Reasons for creating this PR

This PR adds support for new SEO configuration options skosmos:serviceDescription, skosmos:aboutDescription and skosmos:feedbackDescription which make it possible to set a description for the landing, about and feedback pages, respectively. The values are multilingual. If not set, there will be no description metadata on the respective page.

Link to relevant issue(s), if any

Description of the changes in this PR

  • add support for skosmos:serviceDescription, skosmos:aboutDescription and skosmos:feedbackDescription settings in GlobalConfig
  • use the settings to add description metadata to the HTML templates of the landing, about and feedback pages
  • PHPUnit and Cypress tests

Known problems or uncertainties in this PR

There is no Cypress test for the case when one of the descriptions is not set in the configuration and thus should not be displayed. Should there be? The problem here is that this would require a separately configured Skosmos instance with a different configuration that omits some or all of the description settings. We currently only use one Skosmos instance for running Cypress tests, using the tests/testconfig.ttl configuration file.

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)

@osma osma added this to the 3.0 milestone Aug 14, 2024
@osma osma self-assigned this Aug 14, 2024
@osma osma marked this pull request as draft August 14, 2024 10:02
@osma osma force-pushed the issue1533-seo-skosmos3-description branch from 757d205 to c9dd489 Compare August 14, 2024 14:44
Copy link

sonarcloud bot commented Aug 14, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
38.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@osma
Copy link
Member Author

osma commented Aug 14, 2024

Rebased on current main (after merging PR #1644) and force-pushed. Ready for review now.

@osma osma marked this pull request as ready for review August 14, 2024 14:45
@miguelvaara miguelvaara self-requested a review August 15, 2024 09:41
Copy link
Contributor

@miguelvaara miguelvaara left a comment

Choose a reason for hiding this comment

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

All tests (back-end & front-end) passed and the code and implementation look good. I think it can be merged, but maybe you could add descriptions to the Skosmos wiki? Good job!

tests/testconfig.ttl Show resolved Hide resolved
@osma
Copy link
Member Author

osma commented Aug 15, 2024

I added wiki documentation to the checklist in the main SEO issue: #1533 (comment)

Yes, obviously the documentation should explain that these configuration settings are related to SEO.

@osma osma merged commit 9b27db8 into main Aug 15, 2024
9 of 10 checks passed
@osma osma deleted the issue1533-seo-skosmos3-description branch August 15, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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