pluginkollektiv / statify

Statify – statistics plugin for WordPress
https://wordpress.org/plugins/statify/
GNU General Public License v3.0
76 stars 22 forks source link

Widget title addition #230

Open John-H-Smith opened 2 years ago

John-H-Smith commented 2 years ago

Added the title to the widget on hover over url so that you can see what page it is if you have short urls enabled.

image

If it's the front page, the title of the website is returned.

stklcode commented 2 years ago

related to #78

2ndkauboy commented 2 years ago

It seems that the url_to_postid() function (which is also quite heavy) does not really work for many types of URLs, such as CPT single views, CPT archives and even category archives. I'm also a bit unsure if this is a good idea in terms of a11y. :thinking:

John-H-Smith commented 2 years ago

It seems that the url_to_postid() function (which is also quite heavy) does not really work for many types of URLs, such as CPT single views, CPT archives and even category archives. I'm also a bit unsure if this is a good idea in terms of a11y. thinking

Yeah, I thought about that and verified the behaviour with a category archieve. It seems like the function _get_page_by_path_ could be working, but it seems to be quite buggy when switching the permalink structure, so I had switched to _url_topostid. Do you have another function which could be working too?

MatzeKitt commented 2 years ago

I think the cleanest way would be in storing metadata for the visited page in the Statify table, like post ID as an object ID and the object type, which could be any post type or term type. That way you can find the correct object according to object type and object ID and get the current title of the object.

John-H-Smith commented 2 years ago

I think the cleanest way would be in storing metadata for the visited page in the Statify table, like post ID as an object ID and the object type, which could be any post type or term type. That way you can find the correct object according to object type and object ID and get the current title of the object.

Yeah, that would be. But as of that, you would have to update the metadata every time the url or the title is updated - I think thats a quite heavy performance issue.

MatzeKitt commented 2 years ago

But as of that, you would have to update the metadata every time the url or the title is updated - I think thats a quite heavy performance issue.

If you construct it correctly, you can retrieve the current title of all items in the widget with a single SQL query, which is much better than using url_to_postid for every single item.

John-H-Smith commented 1 year ago

If you construct it correctly, you can retrieve the current title of all items in the widget with a single SQL query, which is much better than using url_to_postid for every single item.

Yeah, that's correct, it would be much better. But modifying the table after it has been created would force you to either uninstall and reinstall statify to create the table correctly or check for every website access if the new columns in the table already exist. Also, I just noticed that the dashboard widget limits the count of all items to 100, so I am pretty sure using url_to_postid would be no problem.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

stklcode commented 1 year ago

But modifying the table after it has been created would force you to either uninstall and reinstall statify to create the table correctly [...]

Actually that's not a big deal at all. There is already a routine to initialize the table properly, so the schema may be updated during a plugin update.

MatzeKitt commented 1 year ago

As addition: take a look into dbDelta

John-H-Smith commented 1 year ago

Alright - but with what action hook should I modify the table in your opinion? Directly in the functions file for execution on every call would be a bad practice I think...

MatzeKitt commented 1 year ago

I can only speak for myself, but the best way I found was to store the migration version as option and migrate the database depending on this option.

See here: https://github.com/epiphyt/embed-privacy/blob/main/inc/class-migration.php

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication