patrickdemooij9 / SeoToolkit.Umbraco

SeoToolkit is a SEO package for Umbraco 9, 10, 11, 12 & 13. This package features most functionalities needed for your SEO needs like meta fields, sitemap, robots.txt and much more.
MIT License
34 stars 28 forks source link

Twitter Metadata field support #234

Closed robertjf closed 11 months ago

robertjf commented 11 months ago

This PR Addresses #191 by adding additional Metadata Fields. The Card type can be selected by dropdown.

patrickdemooij9 commented 11 months ago

Looks great! I'll take a better look & test it in the following days and let you know if I have any feedback. But really appreciate this PR and the other one that you did!

robertjf commented 11 months ago

Thanks @patrickdemooij9 - let me know if I can be of assistance; really itching to get this released so we can use it on a couple of projects :)

patrickdemooij9 commented 11 months ago

Hi @robertjf Great PR! Just spotted a few small things. If you want, I can also pick these up but I'll let that decision up to you. Really appreciate the work you do here!

robertjf commented 11 months ago

hey @patrickdemooij9 let me know what they are, and I'll jump on them straight away :)

patrickdemooij9 commented 11 months ago

hey @patrickdemooij9 let me know what they are, and I'll jump on them straight away :)

You should be able to see them above our messages :)

robertjf commented 11 months ago

hm. Not seeing them at all..

robertjf commented 11 months ago

@patrickdemooij9 are they marked as private by any chance?

patrickdemooij9 commented 11 months ago

Hmm, I don't see anything related to private in the mobile app. I'll take a look at it tomorrow in the browser. The feedback was:

Hopefully I'll be able to get review points showing for you tomorrow.

robertjf commented 11 months ago

great - I'll get to work on those in the meantime.

robertjf commented 11 months ago

@patrickdemooij9 I'm about to push up some changes for you - I'm not 100% convinced that the twitter specific fields should be in their own group though; the twitter preview widget is still in the Open Graph group so it's a little disjointed.

Still, I do like the separation. For Facebook I've also added facebook app_id field as well, also in it's own group.

Perhaps we should call the group "Social Media" instead of "Open Graph" and keep it all together?

image

patrickdemooij9 commented 11 months ago

@robertjf You're right, the preview doesn't really fit then. I think a Social Media Group would be a great idea to do then!

robertjf commented 11 months ago

ok, so if we rename the group from Open Graph to Social Media, would that work? and consolidate the fields into that?

patrickdemooij9 commented 11 months ago

Yes, I think that is the way to go about it

robertjf commented 11 months ago

all done :)

patrickdemooij9 commented 11 months ago

PR looks great! I've merged it and I will try to release it somewhere next week. Trying to get a few more changes added for the next version