search

GatherPress / gatherpress

Powering our community's event management needs.
https://wordpress.org/plugins/gatherpress/
GNU General Public License v2.0
86 stars 30 forks source link

Purpose, bug or room for enhancements #557

Open carstingaxion opened 7 months ago

carstingaxion commented 7 months ago

Describe your question

Dear GatherPress Team,

during my translation and while testing the plugin for some days now, some findings came up. I’m unsure if things are done by purpose, by buggy accident or just not polished (enough), so I’d like to ask first and would go on opening issues, if your answers identify my findings as bugs or potential new features.

Let’s start:

An amazing piece of poetry!

Waiting greedy for your answers. Best Carsten

Code of Conduct

mauteri commented 7 months ago

Thanks for this @carstingaxion. Some of these issues may be from ignorance and some are intentional. The code base believe it or not has code that is up to 4 years old, and some areas I probably haven't re-reviewed in years. I'll answer some as best I can, and feel free to put in pull requests to improve areas you think need to be revisited.

Necessity to load JS on the frontend to render an address. This can definitely be changed. I'm not sure why that whole block is set to be dynamic. Only the map really requires it.

Uses fixed non-translatable rewrite slugs for post_types and taxonomy. This should also be updated as this was an international miss on our exclusively American team at the time.

Why are unclosed issues in the DONE column of the project board? @jeffpaul also pointed this out and I have since closed those issues. I didn't realize they remained open when I moved them to Done to be honest. I'll make sure things in Done are closed going forward.

Venue post_type does not take care about the WordPress Geodata Standard. I wasn't familiar with this standard, but this is good to know and to fix. Thanks!

Missing i18n tooling by wp-cli in package.json This should be added. We just started translating the plugin recently with @patriciabt and @javiercasares and we're learning as we go.

Uses non-hierachical venue taxonomy, why? The venue taxonomy is completely programmatic. It's terms are created, updated, and deleted based on the Venue post type. The slug is prepended with a _ and is otherwise the same as the post type. The reason for the two is to allow us to have a presentation page for the venue (the post type) which leverages block editor as well as having the ability to query events by their venue (the taxonomy). This is non-hierarchical since the Venue post type does not have parent, so taxonomy does not need one either.

Weird ´get_taxonomy_registration_args()´ Probably something that could be reworked as I can see that being confusing and not helpful.

Need a way to rename the "Attending" block Do you mean there's some internationalization missing here with the block name?

Dateformat of datetime block is not translated/translatable I think @patriciabt mentioned this as well. Not sure if a ticket was created though.

Does it allow hybrid events? Yup! You can use both a Venue block and an Online Event block. The Online Event block is used in this case of hybrid. In most cases you can use the Venue block and choose Online Event for the venue so it adds the online-event term as the venue.

uses Google Maps by default, which could be a problem in the EU Google Maps was the API we were familiar with when the Venue block was written. We can absolutely support other maps and update that block, especially ones that work better for EU. Also, I didn't realize Google Maps was an issue in EU, so I didn't think much of it.

No location set; Results in Map shows: 60 29th St, San Francisco, CA 94110, USA That SHOULD only be in the admin when nothing is set or could be set (like using the block in FSE template). The address is Automattic's office, as I wanted to use a sample address that was related to WordPress, but didn't cause any issues with privacy.

use block-hooks instead of content filters for the two defined special pages Not sure what this is referring too. Can you elaborate?

mauteri commented 7 months ago

@carstingaxion Let me know what you think of my responses. I'd like to convert some of your checkboxes into issues that we can tackle in 0.29.0. Thanks again!

jeffpaul commented 7 months ago

@jeffpaul also pointed this out and I have since closed those issues. I didn't realize they remained open when I moved them to Done to be honest. I'll make sure things in Done are closed going forward.

You can set up automation on the project board such that when issues move to the Done column that they get closed.

mauteri commented 7 months ago

@carstingaxion we converted a bunch of your questions to issues for folks to work on. 🙌🏻 We'll reach out and chat about some of the others we aren't sure about. Thanks again for bringing these issues up!

pbrocks commented 7 months ago

@carstingaxion Hi there, I'd like to understand more about your thoughts on:

uses Google Maps by default, which could be a problem in the EU

Back in 2018, I contributed to the Privacy team as GDPR was becoming relevant and from my understanding, the issue is about potentially revealing individuals identity. Although we are using Google Maps to draw the map, it's only an iFrame, not an API connection that is doing so. That is, there isn't any registration/consent involved, and as such, it seems that this should circumvent the issue you reference.

However, I understand I am dealing with speculation here, hence couching claims with modals like should and could, so I wonder if you have a suggestion about where to look for a more definitive answer to the question.

patriciabt commented 7 months ago

@pbrocks if the visitor is logged into their google account, it's a visit without the user consent. And anyway why not use an open one instead? see #561

carstingaxion commented 7 months ago

Wow! :tada: and almost :100:

I'll answer some as best I can

What an impressive answer, thank you @mauteri, you did great! I'll have a look into the singular issues, you already created, one by one.

Now I want to clarify some of the unclear aspects of my initial questions.

carstingaxion commented 7 months ago

Some interruptions later ...

I opened a PR that fixes this one missing textdomain string. Now the string is shown correctly translated also within the editor. I hope this is the same, which @patriciabt mentioned, but didn't got an issue.

No, unfortunately its more fundamental. Please have a look at #589

pbrocks commented 7 months ago

@pbrocks if the visitor is logged into their google account, it's a visit without the user consent. And anyway why not use an open one instead? see #561

@patriciabt I'm not against using open source. My point is that, as far as I can tell, there isn't a difference between the way we are using Google Maps and using Open Street Maps. That is, the fact that we haven't created an API connection should present the same situation. If a logged-in Google user visits with either GM (as iframe) or OSM, what can be recorded is the same.

Having said that, we are making the change, but I feel I have to elucidate the situation as I understand it to either be corrected or to clarify. I'm saying there is a difference between using Google to draw the map without an API connection, which would present the issue you raised, and without, which is what GatherPress had done.

Hoping I've got that correct. Happy to be wrong, tho!!

carstingaxion commented 6 months ago

@pbrocks Finally I provided answers to your in https://github.com/GatherPress/gatherpress/issues/639, please have a look and let us discuss what's needed and wanted. Please excuse me being that late!

CC @patriciabt