Skip to content

New plugin infrastructure for Skosmos 3.0#1790

Merged
UnniKohonen merged 21 commits intomainfrom
issue1798-skosmos-3-plugin-infrastructure
May 22, 2025
Merged

New plugin infrastructure for Skosmos 3.0#1790
UnniKohonen merged 21 commits intomainfrom
issue1798-skosmos-3-plugin-infrastructure

Conversation

@UnniKohonen
Copy link
Contributor

@UnniKohonen UnniKohonen commented May 21, 2025 •
edited
Loading

Reasons for creating this PR

Skosmos 3 does not currently have support for plugins. This PR adds new infrastructure for plugins.

Link to relevant issue(s), if any

Description of the changes in this PR

  • Remove support for plugin templates
  • Fix paths to plugin files in plugin registry
  • Add plugins and plugin callbacks to scripts.inc file
  • Add a function to call plugin callbacks on page load
  • Add plugin slot elements to DOM
  • Add test plugins
  • Add cypress tests for plugins

Known problems or uncertainties in this PR

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)

@UnniKohonen UnniKohonen added this to the 3.0 milestone May 21, 2025
@UnniKohonen UnniKohonen self-assigned this May 21, 2025
@UnniKohonen UnniKohonen moved this to In progress in Skosmos 3.x Backlog May 21, 2025
@UnniKohonen UnniKohonen modified the milestones: 3.0, 3.0-alpha.2 May 21, 2025
@codecov
Copy link

codecov bot commented May 21, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.02%. Comparing base (c44f8f7) to head (8cba1eb).
Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
src/model/PluginRegister.php 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff            @@
##               main    #1790   +/-   ##
=========================================
  Coverage     71.02%   71.02%           
  Complexity     1645     1645           
=========================================
  Files            33       33           
  Lines          4314     4314           
=========================================
  Hits           3064     3064           
  Misses         1250     1250

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@UnniKohonen UnniKohonen force-pushed the issue1798-skosmos-3-plugin-infrastructure branch from 3868d51 to 7d216c0 Compare May 21, 2025 07:46
@UnniKohonen UnniKohonen marked this pull request as ready for review May 21, 2025 12:20
@UnniKohonen UnniKohonen requested a review from osma May 21, 2025 13:00
@UnniKohonen UnniKohonen moved this from In progress to Needs review in Skosmos 3.x Backlog May 22, 2025
osma
osma requested changes May 22, 2025
View reviewed changes
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 very good. I gave a few suggestions for potential improvements - these are mainly aspects where we could do better than just copy the solution from Skosmos 2 directly.

@UnniKohonen UnniKohonen requested a review from osma May 22, 2025 10:12
@osma
Copy link
Member

osma commented May 22, 2025

I see that SonarCloud has two complaints about use of var:

var VANILLA_JS_PLUGIN = VANILLA_JS_PLUGIN || {}
Unexpected var, use let or const instead.

(and the same for the Vue plugin)

Can these be fixed as well? Sorry I didn't notice this in my earlier review.

osma
osma reviewed May 22, 2025
View reviewed changes
resource/js/partial-page-load.js
const updateJsonLD = (conceptHTML) => {
const JsonLD = document.querySelector('script[type="application/ld+json"]')
const newJsonLD = conceptHTML.querySelector('script[type="application/ld+json"]')
const JsonLD = document.querySelector('#json-ld-data')
Copy link
Member

@osma osma May 22, 2025

Choose a reason for hiding this comment

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

Suggested change
const JsonLD = document.querySelector('#json-ld-data')
const JsonLD = document.getElementById('json-ld-data')

osma
osma reviewed May 22, 2025
View reviewed changes
resource/js/partial-page-load.js
const JsonLD = document.querySelector('script[type="application/ld+json"]')
const newJsonLD = conceptHTML.querySelector('script[type="application/ld+json"]')
const JsonLD = document.querySelector('#json-ld-data')
const newJsonLD = conceptHTML.querySelector('#json-ld-data')
Copy link
Member

@osma osma May 22, 2025

Choose a reason for hiding this comment

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

Suggested change
const newJsonLD = conceptHTML.querySelector('#json-ld-data')
const newJsonLD = conceptHTML.getElementById('json-ld-data')

Copy link
Contributor Author

@UnniKohonen UnniKohonen May 22, 2025

Choose a reason for hiding this comment

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

getElementById is a method of the document object so it can't be used with the conceptHTML object. This is why I used the querySelector method on both lines.

osma
osma approved these changes May 22, 2025
View reviewed changes
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.

Maybe fix the SonarCloud warnings and consider my suggestions for use of getElementById. Then this is good for merging 🎉

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 22, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@UnniKohonen UnniKohonen merged commit 0ef4a75 into main May 22, 2025
11 of 12 checks passed
@github-project-automation github-project-automation bot moved this from Needs review to Issue/PR closed in Skosmos 3.x Backlog May 22, 2025
@UnniKohonen UnniKohonen deleted the issue1798-skosmos-3-plugin-infrastructure branch May 22, 2025 11:28
@UnniKohonen UnniKohonen moved this from Issue/PR closed to Done (verified in test.dev.finto.fi, set Milestone 3.0 for both issue & PR) in Skosmos 3.x Backlog May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@osma osma osma approved these changes

Assignees

@UnniKohonen UnniKohonen

Labels

enhancement

Projects

None yet

Milestone

3.0-alpha.2

Development

Successfully merging this pull request may close these issues.

New plugin infrastructure for Skosmos 3.0

2 participants

@UnniKohonen @osma