Skip to content

Conversation

bx80
Copy link
Contributor

@bx80 bx80 commented Nov 16, 2023

Description:

Fixes #21365

This PR adds a check to make sure the Feedback plugin assembly is loaded before showing the rate feature icons, this prevents an issue where the feedback plugin is disabled but the cached javascript still shows the rate feature icons which will then be missing translation strings when clicked.

There is a related issue not fixed by this PR where the EnrichedHeadline component in CoreHome depends on the feedback plugin and will always try to load it dynamically when widgets are active, if the feedback plugin is disabled then a browser console error Uncaught (in promise) PluginOnDemandLoadError: Loading plugin Feedback on demand failed. however this does not affect the visible user experience and may require larger changes to fix. Fixed by @sgiehl's suggestion below

Review

@bx80 bx80 added Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Nov 16, 2023
@bx80 bx80 added this to the 5.0.0 milestone Nov 16, 2023
@bx80 bx80 requested review from mneudert and a team November 16, 2023 22:49
@bx80 bx80 self-assigned this Nov 16, 2023
@bx80 bx80 added the Needs Review PRs that need a code review label Nov 17, 2023
bx80 and others added 2 commits November 17, 2023 14:04
@sgiehl
Copy link
Member

sgiehl commented Nov 17, 2023

@bx80 to decouple CoreHome from the Feedback plugin you could try using useExternalPluginComponent, and avoid directly using the component from Feedback plugin. You can see an example here:

testComponent() {
if (this.isJsTrackerInstallCheckAvailable) {
return useExternalPluginComponent('JsTrackerInstallCheck', 'JsTrackerInstallCheck');
}
return '';
},

@bx80
Copy link
Contributor Author

bx80 commented Nov 23, 2023

@sgiehl That worked well and fixes the javascript error when the feedback plugin is disabled. 👍 I've also adjusted the check to see if the feedback plugin is loaded to check directly for the feedback translations rather than the existence of window.Feedback which was still cached in some of my tests.

@bx80 bx80 requested review from sgiehl and michalkleiner November 23, 2023 02:05
@bx80 bx80 requested a review from mneudert November 24, 2023 01:19
@sgiehl sgiehl merged commit 8438f29 into 5.x-dev Nov 24, 2023
@sgiehl sgiehl deleted the m21365-missing-feedback-translations branch November 24, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Development

Successfully merging this pull request may close these issues.

[Bug]: Translation strings missing in 5.0 for user feedback text
4 participants