Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Purpose, bug or room for enhancements #557

Open
10 of 14 tasks
carstingaxion opened this issue Feb 21, 2024 · 10 comments
Open
10 of 14 tasks

Purpose, bug or room for enhancements #557

carstingaxion opened this issue Feb 21, 2024 · 10 comments
Labels
question Further information is requested

Comments

@carstingaxion
Copy link
Collaborator

carstingaxion commented Feb 21, 2024

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

  • I agree to follow this project's Code of Conduct
@carstingaxion carstingaxion added the question Further information is requested label Feb 21, 2024
@mauteri
Copy link
Contributor

mauteri commented Feb 23, 2024

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
Copy link
Contributor

mauteri commented Feb 23, 2024

@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
Copy link
Collaborator

@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
Copy link
Contributor

mauteri commented Feb 23, 2024

@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
Copy link
Collaborator

pbrocks commented Feb 29, 2024

@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
Copy link
Collaborator

@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
Copy link
Collaborator Author

Wow! 🎉 and almost 💯

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.

  • To prepare the mentioned Missing i18n tooling by wp-cli in package.json I opened a PR
  • ....

@carstingaxion carstingaxion mentioned this issue Mar 7, 2024
4 tasks
@carstingaxion
Copy link
Collaborator Author

carstingaxion commented Mar 7, 2024

Some interruptions later ...

  • Dateformat of datetime block is not translated/translatable

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.

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

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

@pbrocks
Copy link
Collaborator

pbrocks commented Mar 7, 2024

@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 carstingaxion changed the title Purpose, bug oder room for enhancements Purpose, bug or room for enhancements Apr 1, 2024
@carstingaxion
Copy link
Collaborator Author

@pbrocks Finally I provided answers to your in #639, please have a look and let us discuss what's needed and wanted. Please excuse me being that late!

CC @patriciabt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants