-
Notifications
You must be signed in to change notification settings - Fork 427
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
Add a Custom TGMPA Generator to the website. #519
Conversation
|
||
<fieldset id="tgmpa-form-wporg"> | ||
<input type="checkbox" id="tgmpa-wporg" name="tgmpa_wporg" value="1"> | ||
<label for="tgmpa-wporg">Do you intend to distribute the theme via wp.org ?</label> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expand to the full wordpress.org
, since wp.org redirects to wpbeginner.com.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh dear, I though that battle had been fought (and won by Automattic). Will change.
Great job! |
Changes all committed. Let's give @Stephen-Cronin and @Otto42 a few days to respond before I squash & we merge. |
0ab8cf0
to
5496f25
Compare
Looks exciting! I'll have a look into this and try to get back to you tomorrow. |
I have a question about what happens when the "Do you intend to distribute via wordpress.org" checkbox is selected. Is the only difference that the code allowing the menu item to be moved from Appearance is removed? If that's the only difference, then we actually want that for ThemeForest themes as well. They need to pass Theme-Check, including the 'add_submenu_page' check. I'm not sure what that means for the option name - "Do you intend to distribute via wordpress.org or ThemeForest" is a bit clunky. The only other thing is the text domain issue. From the table above, the text domain won't be replaced in the class for either .org or non .org themes. Have you had any feedback from @Otto42 or someone else from the .org Theme Review Team on this? I believe that language packs are coming at some point and that using more than one text domain will not be allowed on .org. Various people have mentioned this in the past (eg #474, #218), but now may be a good time to get an update from official sources on when this may be happening. As for ThemeForest themes, I know that translations under the tgmpa text domain are on the roadmap for 3.0 and I agree that that's the neatest solution. However, we are still discussing whether we may or may not prefer the text-domain replaced in the class (even though we don't have the technical driver that .org has). I'll try to get that discussion wrapped up asap. Thanks! |
Hi @Stephen-Cronin, thanks for having a look.
No, see the table. (Strange, seems the headers for the columns had gotten mixed up, corrected now - the more extensive changes are for org themes, the minimal ones for plugins)
Actually it will be. For non-org themes and for plugins, the text domain will only be replaced in the example file. Similarly, for org themes, the load textdomain call we intend to add, will be removed to prevent conflicts with Theme Check. Sorry for the confusion. |
Any reason they couldn't be replaced under both conditions? What benefit is there for keeping |
No reason why we couldn't replace. The current benefit is that we've already got translations ready for some languages. Then again, just thinking, you can have two Not sure how Theme Check deals with two Anyway, this would mean we also wouldn't need to remove the All together, it's easier not to (replace the textdomain) as that particular rabbit hole has got quite some twists and turns. |
Hi @jrfnl Thanks! That makes a lot more sense now (ie the table column headers being mixed up). :) We've finished our discussion and decided that we're all good with the 'tgmpa' text domain remaining in the class (if you decide to keep it that is!) and being loaded (when you add that), etc. It's great you are planning to provide translations and it does make more sense to provide them centrally, rather than having thousands of different themes having to replicate them and keep them up to date etc. However, as mentioned above, we would like the menu location & capability config variables removed and the add_admin_menu() function adjusted to only have add_theme_page(). Which means we don't fit either the .org or non .org columns above - hope that doesn't cause too many problems! |
Hi @Stephen-Cronin, That shouldn't be too hard, let me have a quick look how that'll work out user interface-wise, but the principle of it is easy enough to implement. I'll ping you when I have updated the branch. |
@Stephen-Cronin All done. I've updated the screenshot & the table to reflect the changes made. |
Ok, made some more adjustments. If anyone wants to see what the output of the Generator looks like, I've uploaded the 7 different flavours of output for you to peruse: https://adviesen.stackstorage.com/index.php/s/24rI27wv7p60vyr (will leave them online until this PR is merged) |
02abbaf
to
e01c897
Compare
@jrfnl - Thanks. Table and screenshot look great! Will check out the downloads. |
Language pack support for all themes and plugins already exists on w.org. We don't currently "enforce" it, but we will eventually. Soft sell while we ramp it up, basically. |
Note that we would want themes to use one text domain. Replication of strings is not an issue, since all translations for language packs are centrally managed at translate.w.org. Translate once, everybody gets the benefit. |
@Otto42 Thanks for taking the time to have a look at this and confirming we're on the right track as that's exactly what the Generator is about. For those people publishing their themes via wp.org, we're trying to make it as easy as possible to include TGMPA and still use only one text-domain. The Generator will automatically adjust the TGMPA code to match the current requirements for using the language packs as well as sorting out the For those people not publishing on wp.org (and therefore not getting the benefit of translate.w.org), having TGMPA native translations available will be more efficient. Maintaining different versions of the same (main) repository for different purposes, makes maintenance really hard. This way, we can still cater to the specific needs of wp.org (and Themeforest), without the need for different branches for different uses/publication channels. |
@GaryJones @Stephen-Cronin @Otto42 Just checking: anything which would still need to change ? If not, I'll squash the commits so we can merge. |
a4a954d
to
c27bd2d
Compare
The Custom TGMPA Generator takes the zip file from the latest release (for now: needs to be uploaded after each release as I can't find a hard link to the zip on GitHub) and adjusts the contents based on settings provided by the end-user in a form in the download page of the website and then serves the customized .zip file to the user. The output will be different depending on whether TGMPA will be used within a parent/child-theme or a plugin and the chosen publication channel: wordpress.org, Themeforest or other. Other changes made to the website to support this: * removed most links to the GH downloads and redirected them to the download page to encourage the use of the form. * added a number of javascript libraries used by the custom generator which will conditionally load - i.e. only on the download page. * added the v2.5.2 release zip file to use as base for the generator. * added a blogpost announcing the custom generator
c27bd2d
to
a93cd5b
Compare
Ok, time to get this baby going I'd say ;-) Rebased, squashed & blogpost date updated. Ready for merge. |
Add a Custom TGMPA Generator to the website.
FYI - I'm taking the examples down now the generator is online. If anyone feels they should stay online for whatever reason, let me know. |
The Custom TGMPA Generator takes the zip file from the latest release (for now: needs to be uploaded after each release as I can't find a hard link to the zip on GitHub) and adjusts the contents based on settings provided by the end-user in a form in the download page of the website and then serves the customized .zip file to the user.
Changes made to the website to support this:
This PR presumes the related pull requests to the TGMPA library: #520, #521 and #522 will be merged for the 2.6.0 release, but will already function as is for the v 2.5.2 release.
If Theme developers who publish via wp.org use the Generator to create their package, the Theme Check plugin should (will) no longer trigger warnings on the use of TGMPA.
This should also reduce the number of end-user support requests I've come across in the fora caused by too aggressive search-and-replace actions by theme authors on the class-tgm-plugin-activation.php file.
The Generator will apply the following changes to the TGMPA release package:
add_admin_menu()
function - only have theadd_theme_page()
call in thereload_textdomain
related hook-ins and functions (PR #521)/languages/
directoryIf the file was adjusted, the
@version
tag at the top of the file will also be adjusted to read@version x.x.x for plugin/theme Theme Name/Plugin Name
so it will be easy to spot a file which was adjusted using the custom TGMPA generator.[Update] The version tag will now read
@version x.x.x for plugin/theme Theme Name/Plugin Name for publication on WordPress.org/ThemeForest
if either of those two has been choosen as publication channel.@Stephen-Cronin Are there any specific requirements for the Themeforest themes using TGMPA which are not covered yet ? If so, please let me know and I'll see what I can do.
/cc @Otto42
Resolves #475, resolves #476, resolves #460, resolves #462, resolves #186