matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.92k stars 2.66k forks source link

Add default message when report documentation is not available #22739

Closed ilacftemp closed 20 hours ago

ilacftemp commented 2 weeks ago

Description:

This change addresses the issue where the report documentation appears odd or empty when no documentation is available for a report. Specifically, it ensures that a default message ("No documentation available for this report.") is displayed when reportDocumentation is empty or missing. This improves the user experience by providing clear feedback instead of showing a blank or incomplete UI.

Review

ilacftemp commented 1 week ago

I corrected what you asked for! Thanks for the help @michalkleiner! Could you review this PR again, please?

michalkleiner commented 1 week ago

Fixes #12928

ilacftemp commented 1 week ago

Lint errors corrected! :)

ilacftemp commented 1 week ago

Hi @michalkleiner ! Anything else to correct?

ilacftemp commented 1 week ago

Hey @michalkleiner , I'm sorry for bothering you, but are there any more changes to be made? If there are, I'll work on them right away, otherwise, could you merge this? Thank you so more for your time!

sgiehl commented 2 days ago

I'm personally not sure if adding that default text brings any benefits.

If a documentation is set for such a evolution graph and info icon will be shown on hover: image

Clicking it will show the documentation text: image

So adding a default documentation would cause that the info icon would be shown for every evolution report. Even if no real documentation is available. Clicking on it would then only show the default text, which does not have any additional value for the user.

@ronak-innocraft Do you think adding such a default text would bring any benefits or should we keep the behavior as it is?

ronak-innocraft commented 2 days ago

@ronak-innocraft Do you think adding such a default text would bring any benefits or should we keep the behavior as it is? @sgiehl If there is no useful info that can help user to understand the report than I don't think it makes sense to show help icon by default unless there is targeted help info defined per report.

sgiehl commented 20 hours ago

Hey @ilacftemp. Thanks for putting time and effort into creating this pull request.

As @ronak-innocraft explained, adding this default documentation would not bring any benefit to our users. Therefor we will close this PR unmerged.

If you plan to work on any further change, please consider checking if there is an existing issue for it, so you can comment it or create a new issue for it. That way you are able to get some feedback from our side, if we would consider merging a certain change, before you invest further time on it.

Thanks for your understanding.